Page MenuHomeFreeBSD

ffs: Avoid out-of-bounds accesses in the fs_active bitmap
ClosedPublic

Authored by markj on Dec 22 2020, 8:53 PM.
Tags
None
Referenced Files
F81618349: D27731.id81073.diff
Fri, Apr 19, 1:41 AM
Unknown Object (File)
Wed, Apr 17, 9:47 PM
Unknown Object (File)
Jan 30 2024, 4:53 AM
Unknown Object (File)
Jan 3 2024, 11:53 AM
Unknown Object (File)
Jan 1 2024, 9:10 PM
Unknown Object (File)
Dec 14 2023, 8:23 PM
Unknown Object (File)
Oct 16 2023, 3:46 PM
Unknown Object (File)
Oct 12 2023, 7:36 AM
Subscribers

Details

Reviewers
chs
mckusick
kib
Summary

We use a bitmap to track which cylinder groups have changed between
snapshot creation and filesystem suspension. The "legs" of the bitmap
are four bytes wide (see ACTIVESET()) so we must round up the allocation
size to a multiple of four bytes.

I believe this bug is harmless since UMA/kmem_* will both pad the
allocation and zero the full allocation. Note that malloc() does inline
zeroing when the allocation size is known at compile-time.

Reported by: pho (using KASAN)

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 35631
Build 32526: arc lint + arc unit

Event Timeline

markj requested review of this revision.Dec 22 2020, 8:53 PM

No change in actual running, but definitely correct change.

This revision is now accepted and ready to land.Dec 23 2020, 5:31 AM

In theory, can we have fs_ncg not multiple of 8 ? Even if newfs(8) can only create correctly aligned fs_ncg (but I do not see that it cares), other ways to create filesystem might make it unaligned.

So may be there is a real bug, and fixing it would make rounding fs_ncg up to sizeof(int) without NBBY division.

In D27731#619991, @kib wrote:

In theory, can we have fs_ncg not multiple of 8 ? Even if newfs(8) can only create correctly aligned fs_ncg (but I do not see that it cares), other ways to create filesystem might make it unaligned.

So may be there is a real bug, and fixing it would make rounding fs_ncg up to sizeof(int) without NBBY division.

My concern is unfounded, howmany() rounds up.