mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: NeilBrown <neilb@suse.com>
To: Milan Broz <gmazyland@gmail.com>,
	Mike Snitzer <snitzer@redhat.com>,
	device-mapper development <dm-devel@redhat.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: [PATCH] dm: correctly handle chained bios in dec_pending()
Date: Thu, 15 Feb 2018 20:00:15 +1100	[thread overview]
Message-ID: <87k1vepqzk.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <70cda2a3-f246-d45b-f600-1f9d15ba22ff@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1810 bytes --]


dec_pending() is given an error status (possibly 0)
to be recorded against a bio.
It can be called several times on the one 'struct dm_io',
and it is careful to only assign a non-zero error to
io->status.
However when it then assigned io->status to bio->bi_status,
it is not careful and could overwrite a genuine error status with
zero.

This can happen when chained bios are in use.  If a bio is chained
beneath the bio that this dm_io is handling, the child bio might
complete and set bio->bi_status before the dm_io completes.

This has been possible since chained bios were introduced in 3.14, and
become a lot easier to trigger with commit 18a25da84354 ("dm: ensure bio
submission follows a depth-first tree walk") as that commit caused dm to
start using chained bios itself.

A particular failure mode is that if a bio spans an 'error' target and
a working target, the 'error' fragment will complete instantly and set
the ->bi_status, and the other fragment will normally complete a
little later, and will clear ->bi_status.

The fix is simply to only assign io_error to bio->bi_status when
io_error is not zero.

Reported-and-tested-by: Milan Broz <gmazyland@gmail.com>
Cc: stable@vger.kernel.org (v3.14+)
Signed-off-by: NeilBrown <neilb@suse.com>
---
 drivers/md/dm.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm.c b/drivers/md/dm.c
index d6de00f367ef..68136806d365 100644
--- a/drivers/md/dm.c
+++ b/drivers/md/dm.c
@@ -903,7 +903,8 @@ static void dec_pending(struct dm_io *io, blk_status_t error)
 			queue_io(md, bio);
 		} else {
 			/* done with normal IO or empty flush */
-			bio->bi_status = io_error;
+			if (io_error)
+				bio->bi_status = io_error;
 			bio_endio(bio);
 		}
 	}
-- 
2.14.0.rc0.dirty


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

  parent reply	other threads:[~2018-02-15  9:00 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-14 13:02 DM Regression in 4.16-rc1 - read() returns data when it shouldn't Milan Broz
2018-02-14 20:39 ` NeilBrown
2018-02-14 23:05   ` Mike Snitzer
2018-02-15  0:07     ` [dm-devel] " NeilBrown
2018-02-15  7:37       ` Milan Broz
2018-02-15  8:52         ` NeilBrown
2018-02-15  9:00 ` NeilBrown [this message]
2018-02-15  9:01 ` [RFC PATCH] dm: don't assign zero to ->bi_status of an active bio NeilBrown
2018-02-15  9:09 ` [PATCH] block: be more careful about status in __bio_chain_endio NeilBrown
2019-02-22 21:10   ` Mike Snitzer
2019-02-22 22:46     ` Jens Axboe
2019-02-22 23:55       ` Mike Snitzer
2019-02-23  2:02         ` John Dorminy
2019-02-23  2:44           ` Mike Snitzer
2019-02-23  3:10             ` John Dorminy
2019-06-12  2:56               ` John Dorminy
2019-06-12  7:01                 ` Christoph Hellwig
2019-06-17  7:32                   ` Hannes Reinecke
2018-02-19 13:44 ` DM Regression in 4.16-rc1 - read() returns data when it shouldn't Thorsten Leemhuis
2018-02-19 17:15   ` Mike Snitzer
2018-02-26 10:14     ` Thorsten Leemhuis
2018-02-26 11:01       ` NeilBrown
2018-02-26 17:31         ` Thorsten Leemhuis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k1vepqzk.fsf@notabene.neil.brown.name \
    --to=neilb@suse.com \
    --cc=dm-devel@redhat.com \
    --cc=gmazyland@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=snitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®