linux-kernel - Re: [PATCH v4 3/3] block: prevent race condition on bi_status in __bio_chain

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251202054841.GC15524@lst.de>
Date: Tue, 2 Dec 2025 06:48:41 +0100
From: Christoph Hellwig <hch@....de>
To: Andreas Gruenbacher <agruenba@...hat.com>
Cc: zhangshida <starzhangzsd@...il.com>, Johannes.Thumshirn@....com,
	ming.lei@...hat.com, hsiangkao@...ux.alibaba.com,
	csander@...estorage.com, colyli@...as.com,
	linux-block@...r.kernel.org, linux-bcache@...r.kernel.org,
	linux-kernel@...r.kernel.org, zhangshida@...inos.cn
Subject: Re: [PATCH v4 3/3] block: prevent race condition on bi_status in
 __bio_chain_endio

On Mon, Dec 01, 2025 at 02:07:07PM +0100, Andreas Gruenbacher wrote:
> On Mon, Dec 1, 2025 at 12:25 PM Christoph Hellwig <hch@...radead.org> wrote:
> > On Mon, Dec 01, 2025 at 11:22:32AM +0100, Andreas Gruenbacher wrote:
> > > > -       if (bio->bi_status && !parent->bi_status)
> > > > -               parent->bi_status = bio->bi_status;
> > > > +       if (bio->bi_status)
> > > > +               cmpxchg(&parent->bi_status, 0, bio->bi_status);
> > >
> > > Hmm. I don't think cmpxchg() actually is of any value here: for all
> > > the chained bios, bi_status is initialized to 0, and it is only set
> > > again (to a non-0 value) when a failure occurs. When there are
> > > multiple failures, we only need to make sure that one of those
> > > failures is eventually reported, but for that, a simple assignment is
> > > enough here.
> >
> > A simple assignment doesn't guarantee atomicy.
> 
> Well, we've already discussed that bi_status is a single byte and so
> tearing won't be an issue. Otherwise, WRITE_ONCE() would still be
> enough here.

No.  At least older alpha can tear byte updates as they need a
read-modify-write cycle.  But even on normal x86 the check and the update
would be racy.  The cmpxchg makes the intentions very clear, works
everywhere and given it only happens in the error path does not create
any fast path overhead.