[PATCH 0/1] squashfs: harden sanity check in squashfs_read_xattr_id_table

[Fedor Pchelkin]

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