Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Fedor Pchelkin <pchelkin@ispras.ru>
To:     Phillip Lougher <phillip@squashfs.org.uk>,
        Andrew Morton <akpm@linux-foundation.org>
Cc:     Fedor Pchelkin <pchelkin@ispras.ru>, linux-kernel@vger.kernel.org,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        lvc-project@linuxtesting.org
Subject: [PATCH 0/1] squashfs: harden sanity check in squashfs_read_xattr_id_table
Date:   Tue, 17 Jan 2023 13:52:25 +0300
Message-Id: <20230117105226.329303-1-pchelkin@ispras.ru>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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