linux-kernel - Re: [PATCH] block: Annotate a racy read in blk_do_io

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: <ZkSvKbWj1lhGZvLE@gmail.com>
Date: Wed, 15 May 2024 05:48:41 -0700
From: Breno Leitao <leitao@...ian.org>
To: Marco Elver <elver@...gle.com>, paulmck@...nel.org
Cc: paulmck@...nel.org, Bart Van Assche <bvanassche@....org>,
	Jens Axboe <axboe@...nel.dk>,
	"open list:BLOCK LAYER" <linux-block@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] block: Annotate a racy read in blk_do_io_stat()

On Wed, May 15, 2024 at 09:58:35AM +0200, Marco Elver wrote:
> On Wed, 15 May 2024 at 01:47, Paul E. McKenney <paulmck@...nel.org> wrote:
> > On Mon, May 13, 2024 at 10:13:49AM +0200, Marco Elver wrote:
> > +Use of volatile and __data_racy
> > +-------------------------------
> > +
> > +Adding the volatile keyword to the declaration of a variable causes both
> > +the compiler and KCSAN to treat all reads from that variable as if they
> > +were protected by READ_ONCE() and all writes to that variable as if they
> > +were protected by WRITE_ONCE().
 
> "volatile" isn't something we encourage, right? In which case, I think
> to avoid confusion we should not mention volatile. After all we have
> this: Documentation/process/volatile-considered-harmful.rst

Since you mentioned this document, the other day I was reading
volatile-considered-harmful.rst document, and I was surprised that there
is no reference for READ_ONCE() primitive at all (same for WRITE_ONCE).

	# grep -c READ_ONCE Documentation/process/volatile-considered-harmful.rst
	0

>From my perspective, READ_ONCE() is another way of doing real memory
read (volatile) when you really need, at the same time keeping the
compiler free to optimize and reuse the value that was read.

Should volatile-considered-harmful.rst be also expanded to cover
READ_ONCE()?

Thanks!

--breno