From: NeilBrown <neilb@suse.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: 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>
Cc: linux-block@vger.kernel.org
Subject: [PATCH] block: be more careful about status in __bio_chain_endio
Date: Thu, 15 Feb 2018 20:09:24 +1100 [thread overview]
Message-ID: <87eflmpqkb.fsf@notabene.neil.brown.name> (raw)
In-Reply-To: <70cda2a3-f246-d45b-f600-1f9d15ba22ff@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1427 bytes --]
If two bios are chained under the one parent (with bio_chain())
it is possible that one will succeed and the other will fail.
__bio_chain_endio must ensure that the failure error status
is reported for the whole, rather than the success.
It currently tries to be careful, but this test is racy.
If both children finish at the same time, they might both see that
parent->bi_status as zero, and so will assign their own status.
If the assignment to parent->bi_status by the successful bio happens
last, the error status will be lost which can lead to silent data
corruption.
Instead, __bio_chain_endio should only assign a non-zero status
to parent->bi_status. There is then no need to test the current
value of parent->bi_status - a test that would be racy anyway.
Note that this bug hasn't been seen in practice. It was only discovered
by examination after a similar bug was found in dm.c
Signed-off-by: NeilBrown <neilb@suse.com>
---
block/bio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/bio.c b/block/bio.c
index e1708db48258..ad77140edc6f 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -312,7 +312,7 @@ static struct bio *__bio_chain_endio(struct bio *bio)
{
struct bio *parent = bio->bi_private;
- if (!parent->bi_status)
+ if (bio->bi_status)
parent->bi_status = bio->bi_status;
bio_put(bio);
return parent;
--
2.14.0.rc0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
next prev parent reply other threads:[~2018-02-15 9:09 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 ` [PATCH] dm: correctly handle chained bios in dec_pending() NeilBrown
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 ` NeilBrown [this message]
2019-02-22 21:10 ` block: be more careful about status in __bio_chain_endio 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=87eflmpqkb.fsf@notabene.neil.brown.name \
--to=neilb@suse.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=gmazyland@gmail.com \
--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
Powered by JetHome