[PATCH] dm-integrity: initialize journal accounting in dm_integrity_ctr

[PATCH] dm-integrity: initialize journal accounting in dm_integrity_ctr

[Benjamin Marzinski]

If the superblock for a dm-integrity device is changed while the device
is suspended, it will resume in a failed state, and skip
replay_journal(). If this happens on a journaled dm-integriry table's
first resume, replay_journal() will have never been called, and the
journal will be uninitialized. While nothing can write to the device in
this state, integrity_commit() will be called if it receives a flush
request as well as on suspend. Even through there is nothing to commit,
pad_uncommitted() will check that the journal accounting is sane. This
triggers a kernel warning because the accounting was never set up by
replay_journal(). Fix this by setting free_sectors equal to
journal_entries. Since all the accounting variables are zeroed when
creating the ic structure, setting free_sectors is all that's necessary
to mimic a cleared journal.

Reported-by: syzbot+94dac3c75477e184f0b0@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=94dac3c75477e184f0b0
Fixes: fc79aeb86a2c ("dm-integrity: validate the superblock on resume")
Cc: stable@vger.kernel.org
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 drivers/md/dm-integrity.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index 1659a11a1a29..419a8b958371 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -5430,6 +5430,12 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
 		if (r)
 			goto bad;
 
+		/*
+		 * initialize journal accounting like a cleared journal, to
+		 * handle a changed superblock on resume, where the journal
+		 * is not replayed.
+		 */
+		ic->free_sectors = ic->journal_entries;
 	}
 
 	if (ic->mode == 'B') {
-- 
2.56.0

Re: [PATCH] dm-integrity: initialize journal accounting in dm_integrity_ctr

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] dm-integrity: NULL pointer dereference in dm_integrity_resume()
  [1]
--

commit 8723480dc905caec3ed05a2dfe72dc8d6472ba26
Author: Benjamin Marzinski <bmarzins@redhat.com>

dm-integrity: initialize journal accounting in dm_integrity_ctr

This commit initializes ic->free_sectors to ic->journal_entries in
dm_integrity_ctr() so that journal accounting is valid if the first resume
enters a failed state due to a changed superblock and skips journal replay.

> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 802b377613b06..bb2ec9bad6f0d 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -5410,6 +5410,12 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
>  		if (r)
>  			goto bad;
>  
> +		/*
> +		 * initialize journal accounting like a cleared journal, to
> +		 * handle a changed superblock on resume, where the journal
> +		 * is not replayed.
> +		 */
> +		ic->free_sectors = ic->journal_entries;
>  	}
>  
>  	if (ic->mode == 'B') {

[Severity: High]
This problem wasn't introduced by this patch, but can modifying the
on-disk superblock while suspended also cause dm_integrity_resume() to
dereference a NULL pointer [1]?

In dm_integrity_ctr(), ic->recalc_wq is only allocated when
ic->internal_hash is set, and ic->journal is only allocated when ic->mode
is not 'R' or 'I'.  However, dm_integrity_resume() rereads the superblock
from disk and checks SB_FLAG_DIRTY_BITMAP and SB_FLAG_RECALCULATING
without verifying that those resources were allocated at constructor time:

drivers/md/dm-integrity.c:dm_integrity_resume() {
	...
	if (ic->sb->flags & cpu_to_le32(SB_FLAG_DIRTY_BITMAP)) {
		DEBUG_print("resume dirty_bitmap\n");
		rw_journal_sectors(ic, REQ_OP_READ, 0,
				   ic->n_bitmap_blocks * (BITMAP_BLOCK_SIZE >> SECTOR_SHIFT), NULL);
	...
	if (ic->sb->flags & cpu_to_le32(SB_FLAG_RECALCULATING)) {
		__u64 recalc_pos = le64_to_cpu(ic->sb->recalc_sector);

		DEBUG_print("recalc pos: %llx / %llx\n", recalc_pos, ic->provided_data_sectors);
		if (recalc_pos < ic->provided_data_sectors) {
			queue_work(ic->recalc_wq, &ic->recalc_work);
	...
}

If the superblock is modified while suspended to set SB_FLAG_RECALCULATING
on a target without an internal hash, queue_work() dereferences a NULL
ic->recalc_wq.  Likewise, setting SB_FLAG_DIRTY_BITMAP on an inline-mode
target causes rw_journal_sectors() to index into a NULL ic->journal.

[1] https://sashiko.dev/#/bug/linux-824a598f-b950-4742-b4b6-ae6b1159c25b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111004905.1047753-1-bmarzins@redhat.com?part=1

Re: [PATCH] dm-integrity: initialize journal accounting in dm_integrity_ctr

[Mikulas Patocka]

On Tue, 10 Nov 2026, Benjamin Marzinski wrote:

> If the superblock for a dm-integrity device is changed while the device
> is suspended, it will resume in a failed state, and skip
> replay_journal(). If this happens on a journaled dm-integriry table's
> first resume, replay_journal() will have never been called, and the
> journal will be uninitialized. While nothing can write to the device in
> this state, integrity_commit() will be called if it receives a flush
> request as well as on suspend. Even through there is nothing to commit,
> pad_uncommitted() will check that the journal accounting is sane. This
> triggers a kernel warning because the accounting was never set up by
> replay_journal(). Fix this by setting free_sectors equal to
> journal_entries. Since all the accounting variables are zeroed when
> creating the ic structure, setting free_sectors is all that's necessary
> to mimic a cleared journal.
> 
> Reported-by: syzbot+94dac3c75477e184f0b0@syzkaller.appspotmail.com
> Closes: https://syzbot.org/bug?extid=94dac3c75477e184f0b0
> Fixes: fc79aeb86a2c ("dm-integrity: validate the superblock on resume")
> Cc: stable@vger.kernel.org
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  drivers/md/dm-integrity.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 1659a11a1a29..419a8b958371 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -5430,6 +5430,12 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
>  		if (r)
>  			goto bad;
>  
> +		/*
> +		 * initialize journal accounting like a cleared journal, to
> +		 * handle a changed superblock on resume, where the journal
> +		 * is not replayed.
> +		 */
> +		ic->free_sectors = ic->journal_entries;
>  	}
>  
>  	if (ic->mode == 'B') {
> -- 
> 2.56.0

Hi

I think that we shouldn't attempt to flush the journal at all when the 
device is in erroneous state - because flushing it could cause other 
problems.

I've created a patch that disables journal flushing if the dm-integrity 
device is disabled due to an error - please review it.

Mikulas

Re: [PATCH] dm-integrity: initialize journal accounting in dm_integrity_ctr

[Benjamin Marzinski]

On Wed, Nov 11, 2026 at 03:37:44PM +0100, Mikulas Patocka wrote:
> 
> 
> On Tue, 10 Nov 2026, Benjamin Marzinski wrote:
> 
> > If the superblock for a dm-integrity device is changed while the device
> > is suspended, it will resume in a failed state, and skip
> > replay_journal(). If this happens on a journaled dm-integriry table's
> > first resume, replay_journal() will have never been called, and the
> > journal will be uninitialized. While nothing can write to the device in
> > this state, integrity_commit() will be called if it receives a flush
> > request as well as on suspend. Even through there is nothing to commit,
> > pad_uncommitted() will check that the journal accounting is sane. This
> > triggers a kernel warning because the accounting was never set up by
> > replay_journal(). Fix this by setting free_sectors equal to
> > journal_entries. Since all the accounting variables are zeroed when
> > creating the ic structure, setting free_sectors is all that's necessary
> > to mimic a cleared journal.
> > 
> > Reported-by: syzbot+94dac3c75477e184f0b0@syzkaller.appspotmail.com
> > Closes: https://syzbot.org/bug?extid=94dac3c75477e184f0b0
> > Fixes: fc79aeb86a2c ("dm-integrity: validate the superblock on resume")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  drivers/md/dm-integrity.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> > index 1659a11a1a29..419a8b958371 100644
> > --- a/drivers/md/dm-integrity.c
> > +++ b/drivers/md/dm-integrity.c
> > @@ -5430,6 +5430,12 @@ static int dm_integrity_ctr(struct dm_target *ti, unsigned int argc, char **argv
> >  		if (r)
> >  			goto bad;
> >  
> > +		/*
> > +		 * initialize journal accounting like a cleared journal, to
> > +		 * handle a changed superblock on resume, where the journal
> > +		 * is not replayed.
> > +		 */
> > +		ic->free_sectors = ic->journal_entries;
> >  	}
> >  
> >  	if (ic->mode == 'B') {
> > -- 
> > 2.56.0
> 
> Hi
> 
> I think that we shouldn't attempt to flush the journal at all when the 
> device is in erroneous state - because flushing it could cause other 
> problems.
> 
> I've created a patch that disables journal flushing if the dm-integrity 
> device is disabled due to an error - please review it.

I was on the fence about doing something similar to your method. It does
drop any previous journal writes to the partially filled uncommitted
section, which is a bigger change than just fixing a kernel warning. But
that's the way you think we should go, I'm fine with that.

The way I planned to solve it was (copy-pasted so the whitespace is all
messed up):

diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c              
index 1659a11a1a29..bf817c5ce2b3 100644                                         
--- a/drivers/md/dm-integrity.c                                                 
+++ b/drivers/md/dm-integrity.c                                                 
@@ -2908,7 +2908,7 @@ static void integrity_commit(struct work_struct *w)       
                                                                                
        spin_lock_irq(&ic->endio_wait.lock);                                    
        flushes = bio_list_get(&ic->flush_bio_list);                            
-       if (unlikely(ic->mode != 'J')) {                                        
+       if (unlikely(ic->mode != 'J' || dm_integrity_failed(ic))) {             
                spin_unlock_irq(&ic->endio_wait.lock);                          
                dm_integrity_flush_buffers(ic, true);                           
                goto release_flush_bios;                                        

Even if we don't call dm_integrity_flush_buffers(), any dirty dm_bufio
buffers still still get written out. By calling
dm_integrity_flush_buffers(), we control when they get written out,
and we want to guarantee that they get written out before we suspend.
right?

-Ben

> Mikulas

Re: [PATCH] dm-integrity: initialize journal accounting in dm_integrity_ctr

[Benjamin Marzinski]

On Wed, Nov 11, 2026 at 11:28:17AM -0500, Benjamin Marzinski wrote:
> On Wed, Nov 11, 2026 at 03:37:44PM +0100, Mikulas Patocka wrote:
> > I think that we shouldn't attempt to flush the journal at all when the 
> > device is in erroneous state - because flushing it could cause other 
> > problems.
> > 
> > I've created a patch that disables journal flushing if the dm-integrity 
> > device is disabled due to an error - please review it.
> 
> I was on the fence about doing something similar to your method. It does
> drop any previous journal writes to the partially filled uncommitted
> section, which is a bigger change than just fixing a kernel warning. But
> that's the way you think we should go, I'm fine with that.
> 
> The way I planned to solve it was (copy-pasted so the whitespace is all
> messed up):
> 
> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c              
> index 1659a11a1a29..bf817c5ce2b3 100644                                         
> --- a/drivers/md/dm-integrity.c                                                 
> +++ b/drivers/md/dm-integrity.c                                                 
> @@ -2908,7 +2908,7 @@ static void integrity_commit(struct work_struct *w)       
>                                                                                 
>         spin_lock_irq(&ic->endio_wait.lock);                                    
>         flushes = bio_list_get(&ic->flush_bio_list);                            
> -       if (unlikely(ic->mode != 'J')) {                                        
> +       if (unlikely(ic->mode != 'J' || dm_integrity_failed(ic))) {             
>                 spin_unlock_irq(&ic->endio_wait.lock);                          
>                 dm_integrity_flush_buffers(ic, true);                           
>                 goto release_flush_bios;                                        
> 
> Even if we don't call dm_integrity_flush_buffers(), any dirty dm_bufio
> buffers still still get written out. By calling
> dm_integrity_flush_buffers(), we control when they get written out,
> and we want to guarantee that they get written out before we suspend.
> right?

Never mind. dm_integrity_postsuspend() will already call
dm_integrity_flush_buffers() after the queued commit_work is run, so the
dirty buffers are guaranteed to be written out. I'm fine with your
method (aside from the issue that sashiko caught).

-Ben
 
> -Ben
> 
> > Mikulas