Page MenuHomeFreeBSD

vmapbuf: don't smuggle user address via b_data
ClosedPublic

Authored by brooks on Oct 14 2020, 11:53 PM.

Details

Summary

Instead, add a uaddr argument to vmapbuf. Since this argument is
always a pointer use a type of void * and cast to vm_offset_t in
vmapbuf. (In CheriBSD we've altered vm_fault_quick_hold_pages to
take a pointer and check its bounds.)

In no other situtation does b_data contain a user pointer and vmapbuf
replaces b_data with the actual mapping.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

I know I didn't say this earlier in CheriBSD, but looking at the patch again, I think it might be cleaner to pass in the size explicitly as well and have vmapbuf set bp->bufsize rather than than smuggling that in as well. Together, b_data, b_offset, and b_bufsize describe the memory buffer, and passing in the length would let vmapbuf be responsible for setting all of those.

sys/ufs/ffs/ffs_rawread.c
250 ↗(On Diff #78238)

Unrelated bug: vmapbuf() overwrites bp->b_offset. This should only be setting bp->b_iooffset.

  • Pass size rather than smuggling via b_bufsize.

Thanks.

sys/kern/vfs_bio.c
4915 ↗(On Diff #78430)

This check is now dead code that can be removed I think since size_t is unsigned?

This revision is now accepted and ready to land.Oct 19 2020, 8:25 PM
This revision now requires review to proceed.Oct 19 2020, 9:06 PM

This is good... only caveat is that MFC code near this stuff might be a pain... A minor inconvenience at best, so this is a LGTM!

This revision is now accepted and ready to land.Oct 19 2020, 11:26 PM
This revision was automatically updated to reflect the committed changes.