Page MenuHomeFreeBSD

vm_ooffset_t is now unsigned
ClosedPublic

Authored by vangyzen on Aug 27 2020, 3:28 PM.
Tags
None
Referenced Files
F103047874: D26214.id76285.diff
Wed, Nov 20, 6:12 AM
F103047487: D26214.id76325.diff
Wed, Nov 20, 6:05 AM
Unknown Object (File)
Mon, Nov 4, 3:13 AM
Unknown Object (File)
Mon, Nov 4, 3:13 AM
Unknown Object (File)
Mon, Nov 4, 2:50 AM
Unknown Object (File)
Oct 11 2024, 3:10 AM
Unknown Object (File)
Oct 8 2024, 12:53 AM
Unknown Object (File)
Oct 6 2024, 1:25 AM
Subscribers

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.