Page MenuHomeFreeBSD

vm_ooffset_t is now unsigned
ClosedPublic

Authored by vangyzen on Aug 27 2020, 3:28 PM.

Details

Summary

vm_ooffset_t is now unsigned. Remove some tests for negative values,
or make other adjustments accordingly.

Test Plan

Dead code is hard to test.

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

sys/kern/vfs_vnops.c
2620–2630 ↗(On Diff #76285)

I'm not sure how to update this comment. Suggestions welcome.

sys/fs/tmpfs/tmpfs_subr.c
178 ↗(On Diff #76285)

I think this should be size_t, and before subtraction, the check should be made to return 0.

sys/kern/vfs_vnops.c
2620–2630 ↗(On Diff #76285)

I believe the comment is still valid, it depends on the vm_ooffset_t being type-punned for off_t. Check below (foff > OFF_MAX-size) ensures that we do not require fs to mmap pass OFF_MAX.

vangyzen added inline comments.
sys/kern/vfs_vnops.c
2620–2630 ↗(On Diff #76285)

Makes sense. Thank you.

sys/fs/tmpfs/tmpfs_subr.c
183 ↗(On Diff #76325)

You perform two reads of tmpfs_pages_reserved. If user changes the sysctl meanime, you might get overflow on the second calculation.

vangyzen marked an inline comment as done.
  • fix TOCTOU bug using tmpfs_pages_reserved
This revision is now accepted and ready to land.Aug 31 2020, 9:00 PM
markj added inline comments.
sys/fs/tmpfs/tmpfs_subr.c
183 ↗(On Diff #76325)

To make this pedantically correct I believe you have to use atomic_load_long(&tmpfs_pages_reserved). There is nothing preventing the compiler from loading tmpfs_pages_reserved twice.

This revision was automatically updated to reflect the committed changes.