[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230117105226.329303-1-pchelkin@ispras.ru>
Date: Tue, 17 Jan 2023 13:52:25 +0300
From: Fedor Pchelkin <pchelkin@...ras.ru>
To: Phillip Lougher <phillip@...ashfs.org.uk>,
Andrew Morton <akpm@...ux-foundation.org>
Cc: Fedor Pchelkin <pchelkin@...ras.ru>, linux-kernel@...r.kernel.org,
Alexey Khoroshilov <khoroshilov@...ras.ru>,
lvc-project@...uxtesting.org
Subject: [PATCH 0/1] squashfs: harden sanity check in squashfs_read_xattr_id_table
Hello,
Syzkaller reports the following problem [1].
null-ptr-deref in cache_first_page() happens because 'data[i]' is not
properly initialized in squashfs_read_table(): this is due to its 'length'
argument being zero (thus causing 'pages' being zero, too).
squashfs_read_table() is called twice from squashfs_read_xattr_id_table().
After the first call, 'id_table->xattr_ids' signed value is processed: if
a filesystem is corrupted, I suppose, a negative value is written to it
(in copy_bio_to_actor(), as I understand). So then 'len' and 'indexes'
values become incorrect (in this case, equal to zero), and it causes the
bug. I've added the additional check for '*xattr_ids' not being negative.
But, actually, I don't quite understand why 'xattr_ids' field has
different types in 'struct squashfs_xattr_id_table' and
'struct squashfs_sb_info'. Probably that would be an issue. For example,
in squashfs_xattr_lookup() we compare unsigned 'index' with signed
'msblk->xattr_ids'. So maybe convert 'xattr_ids' field in
'struct squashfs_sb_info' to unsigned type? And that, I think, would fix
the problem described in the patch as well.
[1] https://syzkaller.appspot.com/bug?id=8cf937ba2cde11be769fe8a1330c8d9153e3d7fa
--
Regards,
Fedor
Powered by blists - more mailing lists