]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
isofs: fix KMSAN uninit-value bug in do_isofs_readdir()
authorQasim Ijaz <qasdev00@gmail.com>
Tue, 11 Feb 2025 19:59:00 +0000 (19:59 +0000)
committerJan Kara <jack@suse.cz>
Wed, 12 Feb 2025 13:25:19 +0000 (14:25 +0100)
In do_isofs_readdir() when assigning the variable
"struct iso_directory_record *de" the b_data field of the buffer_head
is accessed and an offset is added to it, the size of b_data is 2048
and the offset size is 2047, meaning
"de = (struct iso_directory_record *) (bh->b_data + offset);"
yields the final byte of the 2048 sized b_data block.

The first byte of the directory record (de_len) is then read and
found to be 31, meaning the directory record size is 31 bytes long.
The directory record is defined by the structure:

struct iso_directory_record {
__u8 length;                     // 1 byte
__u8 ext_attr_length;            // 1 byte
__u8 extent[8];                  // 8 bytes
__u8 size[8];                    // 8 bytes
__u8 date[7];                    // 7 bytes
__u8 flags;                      // 1 byte
__u8 file_unit_size;             // 1 byte
__u8 interleave;                 // 1 byte
__u8 volume_sequence_number[4];  // 4 bytes
__u8 name_len;                   // 1 byte
char name[];                     // variable size
} __attribute__((packed));

The fixed portion of this structure occupies 33 bytes. Therefore, a
valid directory record must be at least 33 bytes long
(even without considering the variable-length name field).
Since de_len is only 31, it is insufficient to contain
the complete fixed header.

The code later hits the following sanity check that
compares de_len against the sum of de->name_len and
sizeof(struct iso_directory_record):

if (de_len < de->name_len[0] + sizeof(struct iso_directory_record)) {
...
}

Since the fixed portion of the structure is
33 bytes (up to and including name_len member),
a valid record should have de_len of at least 33 bytes;
here, however, de_len is too short, and the field de->name_len
(located at offset 32) is accessed even though it lies beyond
the available 31 bytes.

This access on the corrupted isofs data triggers a KASAN uninitialized
memory warning. The fix would be to first verify that de_len is at least
sizeof(struct iso_directory_record) before accessing any
fields like de->name_len.

Reported-by: syzbot <syzbot+812641c6c3d7586a1613@syzkaller.appspotmail.com>
Tested-by: syzbot <syzbot+812641c6c3d7586a1613@syzkaller.appspotmail.com>
Closes: https://syzkaller.appspot.com/bug?extid=812641c6c3d7586a1613
Fixes: 2deb1acc653c ("isofs: fix access to unallocated memory when reading corrupted filesystem")
Signed-off-by: Qasim Ijaz <qasdev00@gmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20250211195900.42406-1-qasdev00@gmail.com
fs/isofs/dir.c

index eb2f8273e6f15e4575d7985024bd97a0f8e0de3a..09df40b612fbf27a1a93af2b4fbf6a607f4a1ab4 100644 (file)
@@ -147,7 +147,8 @@ static int do_isofs_readdir(struct inode *inode, struct file *file,
                        de = tmpde;
                }
                /* Basic sanity check, whether name doesn't exceed dir entry */
-               if (de_len < de->name_len[0] +
+               if (de_len < sizeof(struct iso_directory_record) ||
+                   de_len < de->name_len[0] +
                                        sizeof(struct iso_directory_record)) {
                        printk(KERN_NOTICE "iso9660: Corrupted directory entry"
                               " in block %lu of inode %lu\n", block,