Bug 271310 - potential NULL dereference in fsck_ffs's changeino()
Summary: potential NULL dereference in fsck_ffs's changeino()
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-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-05-08 08:55 UTC by Robert Morris
Modified: 2023-06-08 17:06 UTC (History)
3 users (show)

See Also:


Attachments
a broken FS image that causes a NULL dereference in fsck's changeino() (800.00 KB, application/octet-stream)
2023-05-08 08:55 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-08 08:55:23 UTC
Created attachment 242049 [details]
a broken FS image that causes a NULL dereference in fsck's changeino()

Running fsck_ffs -y on the attached image causes this line in
sbin/fsck_ffs/dir.c changeino() to dereference NULL:

                getinoinfo(dir)->i_depth = depth;

I think the reason is that the link count of the i-node in question
(i-number 3, a directory) has its high bit set, so that it looks
negative in di_nlink (which is int16_t) in this code in checkinode()
in pass1.c:

        if (mode == IFDIR) {
                if (DIP(dp, di_size) == 0) {
                        inoinfo(inumber)->ino_state = DCLEAR;
                } else if (DIP(dp, di_nlink) <= 0) {
                        inoinfo(inumber)->ino_state = DZLINK;
                } else {
                        inoinfo(inumber)->ino_state = DSTATE;
                        cacheino(dp, inumber);
                        countdirs++;
                }

As a result, ino_state is set to DZLINK, and cacheino() is not called,
which is why getinoinfo() eventually returns NULL.

Later, in pass4(), I think the expectation is that ino_linkcnt will be
zero, and clri() will be called. But in fact ino_linkcnt is not zero
(it's negative), so adjust() is called, which effectively expects
cacheino() to have been called.

                        case DZLINK:
                                if (inoinfo(inumber)->ino_linkcnt == 0) {
                                        clri(&idesc, "UNREF", 1);
                                        break;
                                }
                                /* fall through */

                        case FSTATE:
                        case DFOUND:
                                n = inoinfo(inumber)->ino_linkcnt;
                                if (n) {
                                        adjust(&idesc, (short)n);

A backtrace from fsck_ffs -y fsck13a.img :

Program received signal SIGSEGV, Segmentation fault.
Address not mapped to object.
0x000000000020b165 in changeino (dir=3, name=0x201ecf "..", newnum=4, depth=2) at dir.c:712
712                     getinoinfo(dir)->i_depth = depth;
(gdb) where
#0  0x000000000020b165 in changeino (dir=3, name=0x201ecf "..", newnum=4, 
    depth=2) at dir.c:712
#1  0x000000000020a553 in linkup (orphan=3, parentdir=0, name=0x0) at dir.c:664
#2  0x0000000000209acd in adjust (idesc=0x7fffffffe7d0, lcnt=-32765)
    at dir.c:470
#3  0x000000000022025e in pass4 () at pass4.c:94
#4  0x0000000000219ae9 in checkfilesys (filesys=0x7fffffffed79 "junk")
    at main.c:484
#5  0x0000000000218f42 in main (argc=1, argv=0x7fffffffea28) at main.c:210
Comment 1 commit-hook freebsd_committer freebsd_triage 2023-05-27 05:44:38 UTC
A commit in branch main references this bug:

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

commit 03a8680202ef7d7e68adc70625633c490b4ed637
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2023-05-27 05:41:57 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2023-05-27 05:43:21 +0000

    Correct two bugs in fsck_ffs(8) triggered by corrupted filesystems.

    Always create a directory inode structure when a directory inode is
    found in Pass 1 as it is not known whether it will be saved or removed
    in later passes. If it is to be saved the directory inode structure
    is needed to track its status and fsck_ffs(8) will segment fault if
    it does not exist.

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

 sbin/fsck_ffs/pass1.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Comment 2 Kirk McKusick freebsd_committer freebsd_triage 2023-05-27 05:47:06 UTC
Fix checked in. Will close when MFC'ed to 13.
Comment 3 commit-hook freebsd_committer freebsd_triage 2023-06-07 23:16:16 UTC
A commit in branch stable/13 references this bug:

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

commit 52f50301aaabddc6e3c1bc8c354101cfd7ea0463
Author:     Kirk McKusick <mckusick@FreeBSD.org>
AuthorDate: 2023-05-27 05:41:57 +0000
Commit:     Kirk McKusick <mckusick@FreeBSD.org>
CommitDate: 2023-06-07 22:40:35 +0000

    Correct two bugs in fsck_ffs(8) triggered by corrupted filesystems.

    Reported-by:  Robert Morris
    PR:           271310
    PR:           271354
    Sponsored-by: The FreeBSD Foundation

    (cherry picked from commit 03a8680202ef7d7e68adc70625633c490b4ed637)

 sbin/fsck_ffs/pass1.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Comment 4 Kirk McKusick freebsd_committer freebsd_triage 2023-06-08 17:06:13 UTC
MFC'ed to 13.