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

[Benjamin Marzinski <bmarzins@redhat.com>]

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