linux-kernel - RE: [PATCH AUTOSEL 5.0 59/66] fs: stream_open - opener for stream-like files so that read and write can run simultaneously without deadlock

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: <073e5def9e654a1d80cdd79cdcf23361@AcuMS.aculab.com>
Date:   Fri, 26 Apr 2019 11:00:01 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Kirill Smelkov' <kirr@...edi.com>
CC:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Sasha Levin <sashal@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Linux List Kernel Mailing" <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        "Yongzhi Pan" <panyongzhi@...il.com>,
        Jonathan Corbet <corbet@....net>,
        David Vrabel <david.vrabel@...rix.com>,
        Juergen Gross <jgross@...e.com>,
        Miklos Szeredi <miklos@...redi.hu>, Tejun Heo <tj@...nel.org>,
        Kirill Tkhai <ktkhai@...tuozzo.com>,
        Arnd Bergmann <arnd@...db.de>, Christoph Hellwig <hch@....de>,
        Julia Lawall <Julia.Lawall@...6.fr>,
        Nikolaus Rath <Nikolaus@...h.org>,
        Han-Wen Nienhuys <hanwen@...gle.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: RE: [PATCH AUTOSEL 5.0 59/66] fs: stream_open - opener for
 stream-like files so that read and write can run simultaneously without
 deadlock

From: Kirill Smelkov
> Sent: 26 April 2019 08:46
...
> I'm not sure I understand your comment completely, but we convert to
> stream_open only drivers that actually do _not_ use position at all, and
> that were already using nonseekable_open, thus pread and pwrite were
> already returning -ESPIPE for them (nonseekable_open clears
> FMODE_{PREAD,PWRITE} and ksys_{pread,pwrite}64 check for that flag). We
> also convert only drivers that use no_llseek for .llseek, so lseek
> on those files is/was always returning -ESPIPE as well.
> 
> If a driver uses position in its read and write and has support for
> pread/pwrite (FMODE_PREAD and FMODE_PWRITE), pread and pwrite are
> already working _without_ file->f_pos locking - because those system
> calls do not semantically update file->f_pos at all and thus do not take
> file->f_pos_lock - i.e. pread/pwrite can be run simultaneously already.

Looks like I knew that once :-)
Mind you, 'man pread' on my system is somewhat uninformative.

Maybe pread() should always be allowed at offset 0.
Then you wouldn't need all this extra logic.

> If libc implements pread as lseek+read it will work for a single
> user case  (single thread, or fd not shared between processes), but it
> will break because of lseek+read non-atomicity if multiple preads are
> simultaneously used from several threads. And also for such emulation
> for multiple users case there is a chance for pread vs pwrite deadlock,
> since those system calls are using read and write and read and write
> take file->f_pos_lock.

I'd actually rather the pread() failed to compile.
The actual implementation did 3 lseek()s (to save and restore the offset).
A user level emulation could usually get away with one lseek().

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)