* [PATCH] dm-integrity: validate the superblock on resume
2026-09-30 14:25 [syzbot] [dm?] KASAN: wild-memory-access Write in __journal_read_write syzbot
@ 2026-09-30 16:42 ` Mikulas Patocka
0 siblings, 0 replies; 2+ messages in thread
From: Mikulas Patocka @ 2026-09-30 16:42 UTC (permalink / raw)
To: syzbot; +Cc: agk, bmarzins, dm-devel, linux-kernel, snitzer, syzkaller-bugs
On Wed, 30 Sep 2026, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 551c722f4080 Merge tag 'rtc-7.3-fixes' of git://git.kernel..
> git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> console output: https://syzkaller.appspot.com/x/log.txt?x=1111c6c9580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=7d012d9c67977ee4
> dashboard link: https://syzkaller.appspot.com/bug?extid=675c91651049ad042c5f
> compiler: gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=171b0035580000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+675c91651049ad042c5f@syzkaller.appspotmail.com
Hi
Here I'm sending a patch for this bug.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
dm_integrity_resume() re-reads the superblock from the device so that it
picks up the flags and the recalculate position. It performs no
validation on the result, while the constructor validates the superblock
it reads and sizes all the in-memory structures according to it. The user
may modify the superblock on the underlying device while the dm-integrity
device is suspended, so that the two no longer agree.
In particular, access_journal_data() shifts the journal entry index by
ic->sb->log2_sectors_per_block, while ic->journal_section_sectors and the
journal page list were computed with the value that was present at
constructor time. Increasing log2_sectors_per_block makes the index run
past the end of the journal, and dm-integrity then writes 512 bytes
through lowmem_page_address(NULL).
Snapshot the validated superblock in the constructor and refuse to resume
if any of the fields that describe the on-disk geometry changed.
Reported-by: syzbot+675c91651049ad042c5f@syzkaller.appspotmail.com
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Fixes: 118ba36e446c ("dm-integrity: fix recalculation in bitmap mode")
Assisted-by: Claude:claude-opus-5
---
drivers/md/dm-integrity.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
Index: linux-2.6/drivers/md/dm-integrity.c
===================================================================
--- linux-2.6.orig/drivers/md/dm-integrity.c
+++ linux-2.6/drivers/md/dm-integrity.c
@@ -180,6 +180,7 @@ struct dm_integrity_c {
struct dm_bufio_client *bufio;
struct workqueue_struct *metadata_wq;
struct superblock *sb;
+ struct superblock *sb_copy;
unsigned int journal_pages;
unsigned int n_bitmap_blocks;
@@ -3858,6 +3859,35 @@ static void dm_integrity_postsuspend(str
ic->journal_uptodate = true;
}
+/*
+ * The superblock is re-read from the device on every resume, so that we pick
+ * up the flags and the recalculate position. The geometry described by the
+ * superblock must not change though - the in-memory structures (and the
+ * journal in particular) were sized according to the superblock that was
+ * validated in the constructor. Reject a superblock that was modified behind
+ * our back.
+ *
+ * Only the fields that the driver never rewrites may be tested here. In
+ * particular, "version" is recalculated by sb_set_version on every superblock
+ * write and it depends on SB_FLAG_RECALCULATING and SB_FLAG_DIRTY_BITMAP, and
+ * SB_FLAG_DISCARD_KEYED may be set by dm_integrity_resume itself.
+ */
+static bool superblock_changed(struct dm_integrity_c *ic)
+{
+ const __le32 immutable_flags = cpu_to_le32(SB_FLAG_HAVE_JOURNAL_MAC |
+ SB_FLAG_FIXED_PADDING |
+ SB_FLAG_FIXED_HMAC |
+ SB_FLAG_INLINE);
+
+ return memcmp(ic->sb->magic, ic->sb_copy->magic, sizeof(ic->sb->magic)) != 0 ||
+ ic->sb->log2_interleave_sectors != ic->sb_copy->log2_interleave_sectors ||
+ ic->sb->integrity_tag_size != ic->sb_copy->integrity_tag_size ||
+ ic->sb->journal_sections != ic->sb_copy->journal_sections ||
+ ic->sb->log2_sectors_per_block != ic->sb_copy->log2_sectors_per_block ||
+ ((ic->sb->flags ^ ic->sb_copy->flags) & immutable_flags) != 0 ||
+ memcmp(ic->sb->salt, ic->sb_copy->salt, SALT_SIZE) != 0;
+}
+
static void dm_integrity_resume(struct dm_target *ti)
{
struct dm_integrity_c *ic = ti->private;
@@ -3876,6 +3906,18 @@ static void dm_integrity_resume(struct d
if (r)
dm_integrity_io_error(ic, "reading superblock", r);
+ if (unlikely(superblock_changed(ic))) {
+ /*
+ * Restore the superblock that we validated in the constructor,
+ * so that the rest of the driver doesn't operate on values
+ * that don't match the in-memory structures.
+ */
+ memcpy(ic->sb, ic->sb_copy, sizeof(struct superblock));
+ DMERR("The superblock was changed while the device was suspended");
+ dm_integrity_io_error(ic, "superblock check", -EINVAL);
+ goto skip_writes;
+ }
+
if (ic->mode == 'R')
goto skip_writes;
@@ -5427,6 +5469,13 @@ try_smaller_buffer:
ic->just_formatted = true;
}
+ ic->sb_copy = kmemdup(ic->sb, sizeof(struct superblock), GFP_KERNEL);
+ if (!ic->sb_copy) {
+ ti->error = "Cannot allocate superblock copy";
+ r = -ENOMEM;
+ goto bad;
+ }
+
if (!ic->meta_dev && ic->mode != 'I') {
r = dm_set_target_max_io_len(ti, 1U << ic->sb->log2_interleave_sectors);
if (r)
@@ -5525,6 +5574,7 @@ static void dm_integrity_dtr(struct dm_t
kvfree(ic->journal_tree);
if (ic->sb)
free_pages_exact(ic->sb, SB_SECTORS << SECTOR_SHIFT);
+ kfree(ic->sb_copy);
if (ic->internal_shash)
crypto_free_shash(ic->internal_shash);
^ permalink raw reply [flat|nested] 2+ messages in thread