linux-kernel - [RFC v2 06/17] bio: do not check bio->bi

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251221025233.87087-7-agruenba@redhat.com>
Date: Sun, 21 Dec 2025 03:52:21 +0100
From: Andreas Gruenbacher <agruenba@...hat.com>
To: Christoph Hellwig <hch@...radead.org>,
	Jens Axboe <axboe@...nel.dk>,
	Chris Mason <clm@...com>,
	David Sterba <dsterba@...e.com>,
	Satya Tangirala <satyat@...gle.com>
Cc: Andreas Gruenbacher <agruenba@...hat.com>,
	linux-block@...r.kernel.org,
	linux-btrfs@...r.kernel.org,
	linux-raid@...r.kernel.org,
	dm-devel@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: [RFC v2 06/17] bio: do not check bio->bi_status before assigning to it

In a few places, we check if to->bi_status is 0 before setting it.  This
creates the impression of preserving the first error code that occurs,
but that is not actually the case because bio->bi_status is not updated
atomically.  (And even updating bio->bi_status atomically wouldn't
preserve the first error in bio chains longer than two.)  We don't
actually need to preserve the first error code, though; and all that
matters is that when an error occurs, one of the error codes is
preserved.

Checking bio->bi_status before assigning to it might also have been
meant as an optimization.  We are on an error path and the potential
benefit is miniscule, though.

So, don't check bio->bi_status before assigning to it.

Created with Coccinelle using the following semantic patch:

@@
expression status;
struct bio *bio;
@@
- if (status && !bio->bi_status)
-       bio->bi_status = status;
+ bio_set_status(bio, status);

@@
expression status;
struct bio *bio;
@@
- if (status != BLK_STS_OK && bio->bi_status == BLK_STS_OK)
-       bio->bi_status = status;
+ bio_set_status(bio, status);

@@
expression status;
struct bio *bio;
@@
- if (unlikely(status) && !bio->bi_status)
-       bio->bi_status = status;
+ bio_set_status(bio, status);

@@
expression status;
struct bio bio;
@@
- if (status && !bio.bi_status)
-       bio.bi_status = status;
+ bio_set_status(&bio, status);
---
 block/bio.c                  | 3 +--
 block/fops.c                 | 3 +--
 drivers/md/dm-integrity.c    | 3 +--
 drivers/md/dm-zoned-target.c | 3 +--
 drivers/md/md.c              | 6 ++----
 5 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/block/bio.c b/block/bio.c
index 3f408e1ba13f..5389321872f0 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -314,8 +314,7 @@ static struct bio *__bio_chain_endio(struct bio *bio)
 {
 	struct bio *parent = bio->bi_private;
 
-	if (bio->bi_status && !parent->bi_status)
-		parent->bi_status = bio->bi_status;
+	bio_set_status(parent, bio->bi_status);
 	bio_put(bio);
 	return parent;
 }
diff --git a/block/fops.c b/block/fops.c
index b4f911273289..a4a6972cbfbf 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -135,8 +135,7 @@ static void blkdev_bio_end_io(struct bio *bio)
 	bool should_dirty = dio->flags & DIO_SHOULD_DIRTY;
 	bool is_sync = dio->flags & DIO_IS_SYNC;
 
-	if (bio->bi_status && !dio->bio.bi_status)
-		dio->bio.bi_status = bio->bi_status;
+	bio_set_status(&dio->bio, bio->bi_status);
 
 	if (bio_integrity(bio))
 		bio_integrity_unmap_user(bio);
diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c
index e380ed7b2a7b..6d04b067060d 100644
--- a/drivers/md/dm-integrity.c
+++ b/drivers/md/dm-integrity.c
@@ -1616,8 +1616,7 @@ static void dec_in_flight(struct dm_integrity_io *dio)
 			schedule_autocommit(ic);
 
 		bio = dm_bio_from_per_bio_data(dio, sizeof(struct dm_integrity_io));
-		if (unlikely(dio->bi_status) && !bio->bi_status)
-			bio->bi_status = dio->bi_status;
+		bio_set_status(bio, dio->bi_status);
 		if (likely(!bio->bi_status) && unlikely(bio_sectors(bio) != dio->range.n_sectors)) {
 			dio->range.logical_sector += dio->range.n_sectors;
 			bio_advance(bio, dio->range.n_sectors << SECTOR_SHIFT);
diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
index 9da329078ea4..efb960eca8ea 100644
--- a/drivers/md/dm-zoned-target.c
+++ b/drivers/md/dm-zoned-target.c
@@ -77,8 +77,7 @@ static inline void dmz_bio_endio(struct bio *bio, blk_status_t status)
 	struct dmz_bioctx *bioctx =
 		dm_per_bio_data(bio, sizeof(struct dmz_bioctx));
 
-	if (status != BLK_STS_OK && bio->bi_status == BLK_STS_OK)
-		bio->bi_status = status;
+	bio_set_status(bio, status);
 	if (bioctx->dev && bio->bi_status != BLK_STS_OK)
 		bioctx->dev->flags |= DMZ_CHECK_BDEV;
 
diff --git a/drivers/md/md.c b/drivers/md/md.c
index cbca792fa380..5afc6d63aa7b 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9084,8 +9084,7 @@ static void md_end_clone_io(struct bio *bio)
 	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
 		md_bitmap_end(mddev, md_io_clone);
 
-	if (bio->bi_status && !orig_bio->bi_status)
-		orig_bio->bi_status = bio->bi_status;
+	bio_set_status(orig_bio, bio->bi_status);
 
 	if (md_io_clone->start_time)
 		bio_end_io_acct(orig_bio, md_io_clone->start_time);
@@ -9136,8 +9135,7 @@ void md_free_cloned_bio(struct bio *bio)
 	if (bio_data_dir(orig_bio) == WRITE && md_bitmap_enabled(mddev, false))
 		md_bitmap_end(mddev, md_io_clone);
 
-	if (bio->bi_status && !orig_bio->bi_status)
-		orig_bio->bi_status = bio->bi_status;
+	bio_set_status(orig_bio, bio->bi_status);
 
 	if (md_io_clone->start_time)
 		bio_end_io_acct(orig_bio, md_io_clone->start_time);
-- 
2.52.0