Bug 218592
| Summary: | [patch] fsck_ffs(8): incorrect bounds check in preen mode when softdep is enabled | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Todd C. Miller <todd.miller> | ||||
| Component: | misc | Assignee: | freebsd-bugs (Nobody) <bugs> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Some People | CC: | cem, kib, mckusick | ||||
| Priority: | --- | Keywords: | patch | ||||
| Version: | CURRENT | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
The patch looks fine to me. The patch looks fine to me. I'll note that inosused will always be a multiple of CHAR_BIT, so in fact the loop will always work. However, I am a strong believer in having code that will work absent these "known" invariants, so am in favor of the change. (In reply to Kirk McKusick from comment #2) Note that inoused is read from superblock. If we ever get into the situation when the counter becomes negative, should we abort preen mode ? A commit references this bug: Author: kib Date: Fri Apr 14 15:22:01 UTC 2017 New revision: 316852 URL: https://svnweb.freebsd.org/changeset/base/316852 Log: In fsck_ffs pass1, prevent the inosused variable from wrapping. The loop that scans the used inode map when soft updates is in use assumes that the inosused variable is signed. However, ino_t is unsigned, so the loop invariant is incorrect and the check for inosused wrapping to < 0 can never be true. Instead of checking for wrap after the fact just prevent it from happening in the first place. PR: 218592 Submitted by: Todd Miller <todd.miller@courtesan.com> Reviewed by: mckusick MFC after: 1 week Changes: head/sbin/fsck_ffs/pass1.c A commit references this bug: Author: kib Date: Fri Apr 21 10:13:08 UTC 2017 New revision: 317249 URL: https://svnweb.freebsd.org/changeset/base/317249 Log: MFC r316852: In fsck_ffs pass1, prevent the inosused variable from wrapping. PR: 218592 Changes: _U stable/11/ stable/11/sbin/fsck_ffs/pass1.c A commit references this bug: Author: kib Date: Fri Apr 21 10:16:35 UTC 2017 New revision: 317250 URL: https://svnweb.freebsd.org/changeset/base/317250 Log: MFC r316852: In fsck_ffs pass1, prevent the inosused variable from wrapping. PR: 218592 Changes: _U stable/10/ stable/10/sbin/fsck_ffs/pass1.c (In reply to Konstantin Belousov from comment #3) If we ever get into the situation when the counter becomes negative, we should abort preen mode because something is seriously wrong. Suggested fix has been applied and MFC'ed. |
Created attachment 181719 [details] Prevent inosused from wrapping The loop that scans the used inode map when soft updates is in use assumes that the inosused variable is signed. However, ino_t is unsigned, so the loop invariant is incorrect and the check for inosused wrapping to < 0 can never be true. Instead of checking for wrap after the fact it is possible to just prevent it from happening in the first place, which the attached patch does. I've just committed a similar fix to OpenBSD.