Page MenuHomeFreeBSD

geom_gate: ensure readprov is null-terminated

Authored by on Aug 30 2021, 1:48 PM.



With crafted input to the G_GATE_CMD_CREATE ioctl, geom_gate can be made to print kernel memory to the system console, potentially revealing sensitive data from whatever was previously in that memory page.

But but but: this is a case of the sys admin misconfiguring, and you'd need root privileges to do this.

Diff Detail

R10 FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline created this revision.

should this be NAME_MAX+1 instead?
Does NAME_MAX include the null-terminator or not? says it does not include it but then that page is for linux libc... so not sure.


see above. if NAME_MAX does include the null-terminator then a simpler version is to just write \0 to the last byte, no need to copy.

oshogbo added inline comments.

Shouldn't we verify this?


Do you mean to check the return value of snprintf?
The number of characters that would have been printed into output doesn't really tell us much at this point. We know the input is either 1 char long, or 255 legitimate chars, or unbounded potentially-nasty.

Actually thats a good point! snprintf is not a good solution here!
At least snprintf(3) man page suggests that it will keep scanning for a null-terminator. Which means we still have a kernel out of bounds read. Bad.

I suggest a straight forward fixed size memcopy with a single write of \0 to the last byte. if there was an earlier \0 great, if there was not then we'd have done that now.

My preferred fix would be to make all string buffers in 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.

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


this is prob the simplest and safest.
stpncpy may have helped avoid writing unnecessary zeros but that doesnt seem to be available. also this is not a perf critical path, so play it safe.


missed one, dang.
same problem: untrusted input from user mode triggering out-of-bounds read (and crash?).

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

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

I'd feel better about adding a new function if there's a way to properly unit test it (and other functions here). Is there some equivalent of gtest for kernel? Or compile all this stuff into a user mode gtest binary?


this looks fishy at first glance: assigning a stack pointer. but it's temporarily only.
will add a comment.



NAME_MAX does not include the trailing nul.


Style: wrap it to 80 cols.

This revision is now accepted and ready to land.Jan 2 2022, 3:57 AM
This revision now requires review to proceed.Jan 3 2022, 12:27 AM
This revision is now accepted and ready to land.Jan 3 2022, 12:43 AM