Bug 271351 - fsck_ffs can crash if fs_size < fs_ncg*fs_fpg
Summary: fsck_ffs can crash if fs_size < fs_ncg*fs_fpg
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-fs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-10 15:02 UTC by Robert Morris
Modified: 2023-06-08 17:07 UTC (History)
3 users (show)

See Also:


Attachments
broken image that causes fsck_ffs to crash due to fs_size < fs_ncg*fs_fpg (800.00 KB, application/octet-stream)
2023-05-10 15:02 UTC, Robert Morris
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2023-05-10 15:02:25 UTC
Created attachment 242107 [details]
broken image that causes fsck_ffs to crash due to fs_size < fs_ncg*fs_fpg

These two checks in ffs_subr.c's validate_sblock():

                FCHK(fs->fs_size, <=, ((int64_t)fs->fs_ncg - 1) * fs->fs_fpg,
                    %jd);

and 

        FCHK(fs->fs_size, <=, ((int64_t)fs->fs_ncg - 1) * fs->fs_fpg, %jd);

allow fs_size to be smaller than fs_ncg*fs_fpg. For example, if fs_ncg
is 1, then the test only requires fs_size to be > 0.

validate_sblock() checks fs_csaddr against fs_ncg*fs_fpg, and thus
fs_csaddr can point beyond fs_size.

As a result, a too-large value of fs_csaddr can cause this code in
fsck_ffs's pass1() to write beyond the end of blockmap, whose size is
determined by fs_size:

        i = sblock.fs_csaddr;
        cgd = i + howmany(sblock.fs_cssize, sblock.fs_fsize);
        for (; i < cgd; i++)
                setbmap(i);

I've attached a file system image that causes fsck_ffs -y to either
dump core or fail valgrind.
Comment 1 commit-hook freebsd_committer freebsd_triage 2023-05-27 19:24:51 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=c79a1416955a260424a5dd2013b114ff864bc926

commit c79a1416955a260424a5dd2013b114ff864bc926
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2023-05-27 19:23:39 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2023-05-27 19:24:11 +0000

    Updates to UFS/FFS superblock integrity checks when reading a superblock.

    Verify that the summary information does not extend past the end
    of the filesystem.

    No legitimate superblocks should fail as a result of these changes.

    Reported-by:  Robert Morris
    PR:           271351
    MFC-after:    1 week
    Sponsored-by: The FreeBSD Foundation

 sys/ufs/ffs/ffs_subr.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
Comment 2 Kirk McKusick freebsd_committer freebsd_triage 2023-05-27 19:32:31 UTC
Fix checked in. Will close when MFC'ed to 13.
Comment 3 commit-hook freebsd_committer freebsd_triage 2023-06-07 23:16:18 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=95fc911fe9f258ab9baa6b9fa1b1643f86b103b6

commit 95fc911fe9f258ab9baa6b9fa1b1643f86b103b6
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2023-06-07 23:12:12 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2023-06-07 23:12:12 +0000

    Updates to UFS/FFS superblock integrity checks when reading a superblock.

    Reported-by:  Robert Morris
    PR:           271351
    Sponsored-by: The FreeBSD Foundation

    (cherry picked from commit c79a1416955a260424a5dd2013b114ff864bc926)

 sys/ufs/ffs/ffs_subr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 4 Kirk McKusick freebsd_committer freebsd_triage 2023-06-08 17:07:33 UTC
MFC'ed to 13.