linux-ext4 - Re: [RFC] Fine-grained locking documentation for jbd2 data structures

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]

Date:   Tue, 9 Feb 2021 17:48:16 +0100
From:   Jan Kara <jack@...e.cz>
To:     Alexander Lochmann <alexander.lochmann@...dortmund.de>
Cc:     Jan Kara <jack@...e.cz>, tytso@....edu, Jan Kara <jack@...e.com>,
        Horst Schirmeier <horst.schirmeier@...dortmund.de>,
        linux-ext4@...r.kernel.org
Subject: Re: [RFC] Fine-grained locking documentation for jbd2 data structures

On Tue 09-02-21 14:47:28, Alexander Lochmann wrote:
> On 09.02.21 13:00, Jan Kara wrote:
> > > > Yes, although in last year, people try to convert these unlocked reads to
> > > > READ_ONCE() or similar as otherwise the compiler is apparently allowed to
> > > > generate code which is not safe. But that's a different story.
> > > Is this ongoing work?
> > 
> > Yes, in a way. It's mostly prompted by KCSAN warnings generated by syzbot
> > ;).
> > 
> > > Using such a macro would a) make our work much easier as we can instrument
> > > them, and b) would tell less experienced developers that no locking is
> > > needed.
> > 
> > Yes, I agree that it has some benefit for documentation and automatic
> > checkers as well. OTOH code readability is sometimes hurt by this...
> > 
> > > Does the usage of READ_ONCE() imply that no lock is needed?
> > 
> > No, but it does indicate there's something unusual happening with the
> > variable - usually that variable write can race with this read.
> > 
> > > Otherwise, one could introduce another macro for jbd2, such as #define
> > > READ_UNLOCKED() READ_ONCE(), which is more precise.
> > 
> > Well, yes, but OTOH special macros for small subsystems like this are
> > making more harm than good in terms of readability since people have to
> > lookup what exactly they mean anyway.
>
> So the only option left would be a global macro such as READ_ONCE() I guess.
> How hard would it be to establish such a global notation?
> It would make things a lot easier for LockDoc, because we can instrument
> such a macro, and therefore can annotate those accesses.>

Well, READ_ONCE() macro already exists in the kernel and is used in quite
some places. Generally, there's concensus that newly added unlocked
accesses should use that macro. But the amount of already existing unlocked
accesses in the kernel is large and the problem is mostly theoretical / for
machine checkers so there's some resistance to the churn generated by
global search-and-replace approach. But as I said we are moving in that
direction and eventually get there.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR