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 - subversion
Lint
Lint Not Applicable
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.