linux-kernel - Re: [RFC 00/12] bio cleanups

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] [day] [month] [year] [list]

Message-ID: <CAHc6FU6vax1eNB-xrYLuZX5s-RLRhtctG7=3NO+h_GPj5o=W-Q@mail.gmail.com>
Date: Mon, 8 Dec 2025 22:16:44 +0100
From: Andreas Gruenbacher <agruenba@...hat.com>
To: dsterba@...e.cz
Cc: Christoph Hellwig <hch@...radead.org>, Jens Axboe <axboe@...nel.dk>, Chris Mason <clm@...com>, 
	David Sterba <dsterba@...e.com>, Satya Tangirala <satyat@...gle.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: Re: [RFC 00/12] bio cleanups

On Mon, Dec 8, 2025 at 8:43 PM David Sterba <dsterba@...e.cz> wrote:
> On Mon, Dec 08, 2025 at 12:10:07PM +0000, Andreas Gruenbacher wrote:
> > 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?
>
> The btrfs bits look good to me, we expect the same semantics, ie. not
> overwrite existing error with 0. If there are racing writes to the
> status like in btrfs_bio_end_io() we use cmpxchg() so we don't overwrite
> it.

Really? I'm not talking about the status field in struct btrfs_bio but
about the bi_status field in struct bio. The first mention of
bi_status I can find in the btrfs code is right at the beginning of
btrfs_bio_end_io():

  bbio->bio.bi_status = status;

If status is ever BLK_STS_OK (0) here and bbio->bio is a chained bio,
things are already not great.

I believe we should eliminate all direct assignments to bi_status and
use bio_set_status() instead. I'm not familiar enough with the btrfs
code to make that replacement for you, though.

A cursory look at struct btrfs_bio suggests that those objects cannot
be chained like plain old bios (bio_chain()). This means that
cmpxchg() may actually work for catching the first error that occurs.
For chained regular bios, cmpxchg() won't catch the first error, at
least not if the length of the chain is greater than two.

Thanks,
Andreas