[PATCH] dm-integrity: don't attempt to commit journal on disabled device

[PATCH] dm-integrity: don't attempt to commit journal on disabled device

[Mikulas Patocka]

If the device is disabled due to an error, we shouldn't attempt to flush 
the journal. We jump straight to the label release_flush_bios, that will 
call do_endio on all queued bios. do_endio will terminate the bios with 
the error.

This fixes WARN_ON in the function pad_uncommitted reported by syzkaller.

Reported-by: syzbot+94dac3c75477e184f0b0@syzkaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=94dac3c75477e184f0b0
Fixes: 7eada909bfd7 ("dm: add integrity target")
Cc: stable@vger.kernel.org
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 drivers/md/dm-integrity.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

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
@@ -2908,6 +2908,10 @@ static void integrity_commit(struct work
 
 	spin_lock_irq(&ic->endio_wait.lock);
 	flushes = bio_list_get(&ic->flush_bio_list);
+	if (unlikely(dm_integrity_failed(ic))) {
+		spin_unlock_irq(&ic->endio_wait.lock);
+		goto release_flush_bios;
+	}
 	if (unlikely(ic->mode != 'J')) {
 		spin_unlock_irq(&ic->endio_wait.lock);
 		dm_integrity_flush_buffers(ic, true);
@@ -5409,7 +5413,6 @@ try_smaller_buffer:
 		r = create_journal(ic, &ti->error);
 		if (r)
 			goto bad;
-
 	}
 
 	if (ic->mode == 'B') {

Re: [PATCH] dm-integrity: don't attempt to commit journal on disabled device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] dm-integrity: missed wakeup of endio_wait in
  integrity_commit() on error
--

commit 6f78b56af54c9b05dda8dbe4a7aa354299dc4892
Author: Mikulas Patocka <mpatocka@redhat.com>

dm-integrity: don't attempt to commit journal on disabled device

This commit skips committing the journal in integrity_commit() when
dm_integrity_failed() is true, jumping directly to release_flush_bios.

> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
> index 802b377613b06..c50fcf2212365 100644
> --- a/drivers/md/dm-integrity.c
> +++ b/drivers/md/dm-integrity.c
> @@ -2908,6 +2908,10 @@ 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(dm_integrity_failed(ic))) {
> +		spin_unlock_irq(&ic->endio_wait.lock);
> +		goto release_flush_bios;
> +	}

[Severity: Medium]
Can this leave tasks sleeping on ic->endio_wait hung forever?

In journal mode, when ic->free_sectors reaches 0, a write bio offloaded to
ic->wait_wq calls sleep_on_endio_wait() in dm_integrity_map_continue() and
waits on ic->endio_wait before looping back to retry: and rechecking
dm_integrity_failed():

drivers/md/dm-integrity.c:dm_integrity_map_continue() {
	...
			if (unlikely(!dio->range.n_sectors)) {
				if (from_map)
					goto offload_to_thread;
				sleep_on_endio_wait(ic);
				goto retry;
			}
	...
}

Previously, even after dm_integrity_failed() became non-zero,
integrity_commit() still advanced uncommitted sections to
ic->n_committed_sections and queued ic->writer_work, allowing
integrity_writer() to replenish ic->free_sectors and call
wake_up_locked(&ic->endio_wait).

Jumping directly to release_flush_bios without waking ic->endio_wait or
advancing uncommitted sections means integrity_writer() is never scheduled
if an I/O or checksum error occurs while the journal is full.  The worker
in sleep_on_endio_wait() then blocks indefinitely on ic->wait_wq, stalling
subsequent bios as well as device suspend or teardown.

Calling wake_up_locked(&ic->endio_wait) before unlocking
ic->endio_wait.lock would let any sleeping writer wake up and fail the bio
at retry:.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/fe63f092-ef07-ac3f-7878-22083541f79e@redhat.com?part=1