In D31727#717074, @philip wrote:

I wonder if this wants some abstraction in the form of a function rather than chasing every str*cpy...?

Handle oversized write buffer coming from zfs

Try to avoid out-of-bounds reads that can happen with other str*cpy functions.

My preferred fix would be to make all string buffers in https://github.com/freebsd/freebsd-src/blob/main/sys/geom/gate/g_gate.h#L118 NAME_MAX+1 and enforce the last byte to be the null terminator.
But that'd be a ioctl interface change and an ABI change? Frowned upon?
Also this mainly affects hastd, which I cannot test at the moment.

follow up in https://reviews.freebsd.org/D31722

Add test for ggatec to check handling of non-read/write request (e.g. trim).

there's a rebase in https://github.com/bruelltuete/freebsd-src/commit/999394f2930d341b422a5421ff931dfb024eefc7, if that's easy to use (as a patch).
i dont actually know how to commit anything to src, never done that before.
do you commit via phabricator, like click a button here in the ui somewhere? does the patch in this review need to be updated?

the security patch has landed. do we still want the tests?

Thanks for review and comments, everyone!

I'm still on 12-stable with all my machines, and will not be able to upgrade for a few more months...

Yeah lots of stuff broken here. We can fix it all in a later patch. For now I really want to plug that remote code execution hole.

add test

change size check and fix typo

In D31318#705648, @jo_bruelltuete.com wrote:

Maybe found another problem, this time with the ggate kernel module. I'm pretty unsure about some details, esp around ioctl, but here goes:
I think it's possible to craft ioctl with ggate to exfiltrate kernel memory.

1. Right now ggatec sends commands (and receives them) by sending an ioctl to the ggctl device. It passes a pointer to a structure that matches on the kernel side. Notice that the ioctl does not have a size parameter, so cannot copyin the data. Instead the ggate-ioctl handler receives the corresponding user mode address as a direct-map address, https://github.com/freebsd/freebsd-src/blob/main/sys/geom/gate/g_gate.c#L736 can be see by setting kern.geom.gate.debug=4.

2. lets say we are doing a G_GATE_CMD_CREATE. https://github.com/freebsd/freebsd-src/blob/main/sys/geom/gate/g_gate.c#L465. The ioctl structure has strings as arrays embedded, https://github.com/freebsd/freebsd-src/blob/main/sys/geom/gate/g_gate.h#L120.

3. by filling gctl_readprov member with garbage and not null terminating we can trigger a printf of its contents: https://github.com/freebsd/freebsd-src/blob/main/sys/geom/gate/g_gate.c#L553. So far this is only our own data.

4. But we can position this structure at the end of a page and fill the remaining members with non-zeros. The following G_GATE_DEBUG will leak the contents of the next physical page into the system console.

5. That next physical page can be from a different process! The address passed in to the ioctl handler is from the direct map.

Does that sound about right?

A quick fix would be to check that gctl_readprov is null terminated.

Maybe found another problem, this time with the ggate kernel module. I'm pretty unsure about some details, esp around ioctl, but here goes:
I think it's possible to craft ioctl with ggate to exfiltrate kernel memory.

While I have your attention here, can someone assign the bug reports linked in the description to geom@freebsd.org? Right now they are dangling assigned to nobody.

What do we want the test to verify? That the fixed version only supports reads and writes? That feels a bit limiting because adding bio_delete or bio_flush support looks pretty straight forward.
Any opinions?

Re testing, it's easy to repro if you can generate a request that is not read or write. I don't know of a reliable way to do that. At the moment zpool-create does that... but only if the right sysctls are set?
And looking at https://github.com/freebsd/freebsd-src/blob/main/tests/sys/geom/class/gate/ggate_test.sh, I'm not sure what to make of that, it's not obvious whats going on with the tests...

Fix still relevant to current: https://github.com/freebsd/freebsd-src/blob/main/sbin/ggate/ggatec/ggatec.c

I'm still working on this but it's really slow going rn, got very little time.

I think i've wrapped my head around dmap & friends... it makes things more complicated.
Will have to demote dmap mappings. If i try to encrypt user-mode pages first, i'll need to demote the dmap pages down from 1gb into 4k and set the c-bit for the small pages.
Same for trying to encrypt all pages. Will still have to demote dmap pages because of dma and the existing kernel text and preloaded module pages.

hi kib, thanks for taking a look!
i'll try to figure out the dmap bits, thanks for the pointer.

more context