linux-kernel - Re: KCSAN: data-race in generic_file_buffered_read / generic_file_buffered

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: <20200716131929.GL12769@casper.infradead.org>
Date:   Thu, 16 Jul 2020 14:19:29 +0100
From:   Matthew Wilcox <willy@...radead.org>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     Nicholas Piggin <npiggin@...il.com>,
        Dave Chinner <david@...morbit.com>, akpm@...ux-foundation.org,
        Marco Elver <elver@...gle.com>, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org,
        "Paul E. McKenney" <paulmck@...nel.org>,
        syzbot <syzbot+0f1e470df6a4316e0a11@...kaller.appspotmail.com>,
        syzkaller-bugs@...glegroups.com, Will Deacon <will@...nel.org>
Subject: Re: KCSAN: data-race in generic_file_buffered_read /
 generic_file_buffered_read

On Wed, Jul 15, 2020 at 11:54:54PM -0700, Eric Biggers wrote:
> > >> > Concurrent reads on the same file descriptor are allowed.  Not with sys_read(),
> > >> > as that implicitly uses the file position.  But it's allowed with sys_pread(),
> > >> > and also with sys_sendfile() which is the case syzbot is reporting here.
> > >> 
> > >> Concurrent read()s are fine, they'll just read from the same offset.
> > >> 
> > > 
> > > Actually the VFS serializes concurrent read()'s on the same fd, at least for
> > > regular files.
> > 
> > Hmm, where?
> 
> It's serialized by file->f_pos_lock.  See fdget_pos().

What if we trylock either f_lock or f_pos_lock for readahead and just
skip all the readahead code if it's already taken?  I'd suggest that if
there are two readers using the same struct file, this is probably not
a workload that benefits greatly from readahead.