| 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
| ||
|
Message-ID: <20251208121020.1780402-1-agruenba@redhat.com>
Date: Mon, 8 Dec 2025 12:10:07 +0000
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 00/12] bio cleanups
Hello,
we are not quite careful enough about setting bio->bi_status in all
places (see BACKGROUND below). This patch queue tries to fix this by
systematically eliminating the direct assignments to bi_status sprinkled
all throughout the code. Please comment.
The first patch ("bio: rename bio_chain arguments") is an loosely
related cleanup. The remaining changes are:
- Use bio_io_error() in more places.
- Add a bio_set_errno() helper for setting bi_status based on an errno.
Use this helper throughout the code.
- Add a bio_set_status() helper for setting bi_status to a blk_status_t
status code. Use this helper in places in the code where it's
necessary, or at least useful without adding any overhead.
And on top of that, we have two more cleanups:
- Add a bio_endio_errno() helper that combines bio_set_errno() and
bio_endio().
- Add a bio_endio_status() helper that combines bio_set_status() and
bio_endio().
The patches are currently based on v6.18.
GIT tree:
https://git.kernel.org/pub/scm/linux/kernel/git/gfs2/linux-gfs2.git/log/?h=bio-cleanups
With these changes, only a few direct assignments to bio->bi_status
remain, in BTRFS and in MD, and SOME OF THOSE MAY BE UNSAFE. Could the
maintainers of those subsystems please have a look?
Once the remaining direct assignments to bi_status are gone, we may want
to think about "write protecting" bi_status to prevent unintended new
direct assignments from creeping back in.
BACKGROUND
'struct bio' objects start out with their bi_status field initialized to
BLK_STS_OK (0). When the bio completes, that field needs to be left
unchanged in case of success, and set to a BLK_STS_* error code
otherwise.
This is important when bios are chained (bio_chain()) because then,
multiple execution contexts will race updating the same bi_status field.
When an execution context resets bi_status to BLK_STS_OK (0) during bio
completion, this could hide the error code of the adjacent bio in the
chain.
When more than a single bio fails in a chain, we know that the resulting
bi_status will not be BLK_STS_OK, but we don't know which of the status
error codes will win.
CRYPTO FALLBACK (SATYA TANGIRALA?)
Related to chained bios but unrelated to setting bio->bi_status,
blk_crypto_fallback_encrypt_bio() in block/blk-crypto-fallback.c swaps
out bi_private and bi_end_io and reuses the same bio for downcalls, then
restores those fields in blk_crypto_fallback_decrypt_endio() before
calling bio_endio() again on the same bio. This will at the very least
break with chained bios because it will mess up __bi_remaining.
Thanks,
Andreas
Andreas Gruenbacher (12):
bio: rename bio_chain arguments
bio: use bio_io_error more often
bio: add bio_set_errno
bio: use bio_set_errno in more places
bio: add bio_set_status
bio: don't check target->bi_status on error
bio: use bio_set_status for BLK_STS_* status codes
bio: use bio_set_status in some more places
bio: switch to bio_set_status in submit_bio_noacct
bio: never set bi_status to BLK_STS_OK during completion
bio: add bio_endio_errno
bio: add bio_endio_status
block/bio-integrity-auto.c | 3 +--
block/bio.c | 25 ++++++++++++-------------
block/blk-core.c | 10 ++++------
block/blk-crypto-fallback.c | 22 +++++++++++-----------
block/blk-crypto-internal.h | 2 +-
block/blk-crypto.c | 4 ++--
block/blk-merge.c | 6 ++----
block/blk-mq.c | 11 ++++-------
block/fops.c | 6 ++----
block/t10-pi.c | 2 +-
drivers/block/aoe/aoecmd.c | 8 ++++----
drivers/block/aoe/aoedev.c | 2 +-
drivers/block/drbd/drbd_int.h | 3 +--
drivers/block/drbd/drbd_req.c | 9 +++------
drivers/block/ps3vram.c | 3 +--
drivers/block/zram/zram_drv.c | 4 ++--
drivers/md/bcache/bcache.h | 3 +--
drivers/md/bcache/request.c | 8 +++-----
drivers/md/dm-cache-target.c | 9 +++++----
drivers/md/dm-ebs-target.c | 2 +-
drivers/md/dm-flakey.c | 2 +-
drivers/md/dm-integrity.c | 30 +++++++++++-------------------
drivers/md/dm-mpath.c | 6 ++----
drivers/md/dm-pcache/dm_pcache.c | 3 +--
drivers/md/dm-raid1.c | 7 +++----
drivers/md/dm-thin.c | 5 ++---
drivers/md/dm-vdo/data-vio.c | 3 +--
drivers/md/dm-verity-target.c | 2 +-
drivers/md/dm-writecache.c | 7 +++----
drivers/md/dm-zoned-target.c | 2 +-
drivers/md/dm.c | 4 +---
drivers/md/md.c | 8 +++-----
drivers/md/raid1-10.c | 3 +--
drivers/md/raid1.c | 2 +-
drivers/md/raid10.c | 18 +++++++-----------
drivers/md/raid5.c | 4 ++--
drivers/nvdimm/btt.c | 4 ++--
drivers/nvdimm/pmem.c | 7 ++-----
fs/btrfs/bio.c | 8 ++++----
fs/btrfs/direct-io.c | 2 +-
fs/btrfs/raid56.c | 6 ++----
fs/crypto/bio.c | 2 +-
fs/erofs/fileio.c | 2 +-
fs/erofs/fscache.c | 4 ++--
fs/f2fs/data.c | 6 +++---
fs/f2fs/segment.c | 3 +--
fs/iomap/ioend.c | 3 +--
fs/verity/verify.c | 2 +-
fs/xfs/xfs_aops.c | 3 +--
fs/xfs/xfs_zone_alloc.c | 2 +-
include/linux/bio.h | 30 +++++++++++++++++++++++++++---
51 files changed, 153 insertions(+), 179 deletions(-)
--
2.51.0
Powered by blists - more mailing lists