linux-kernel - Re: [PATCH] [RFC] Fix deadlock on regular nonseekable files

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]

Date:	Fri, 20 Mar 2015 20:00:52 +0100
From:	Marek Marczykowski-Górecki 
	<marmarek@...isiblethingslab.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Vitaly Chernooky <vitalii.chernookyi@...ballogic.com>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	David Vrabel <david.vrabel@...rix.com>,
	Iurii Konovalenko <iurii.konovalenko@...ballogic.com>,
	Ian Campbell <ian.campbell@...rix.com>,
	Boris Ostrovsky <boris.ostrovsky@...cle.com>,
	Andrii Anisov <andrii.anisov@...ballogic.com>,
	Artem Mygaiev <artem.mygaiev@...ballogic.com>
Subject: Re: [PATCH] [RFC] Fix deadlock on regular nonseekable files

On Fri, Mar 20, 2015 at 05:55:17PM +0000, Al Viro wrote:
> On Fri, Mar 20, 2015 at 07:37:58PM +0200, Vitaly Chernooky wrote:
> > > What does it have to do with filesystem type involved?
> > 
> > Discussed trouble directly depends on is it used nonseekable_open() in
> > fs driver or not. Regular fss like ext4 does not use
> > nonseekable_open() so there is no troubles. But others like ubifs,
> > debugfs, fuse and xenfs use it and are affected by discussed
> > regression.
> 
> What does nonseekable_open() have to do with that, other than as
> a heuristics for "we want to break 2.9.7"?
> 
> > I do not know who, when and why has introduced this
> > mess. It is pre-git historical code. And now by introducing
> > FMODE_ATOMIC_POS we change behavior of this mature practice and affect
> > at least few filesystems. Yes, following standards is good, but I do
> > not accept than breaking mature code is good idea. So I have chosen to
> > clear FMODE_ATOMIC_POS in nonseekable_open() simultaneously with
> > FMODE_LSEEK because it returns that nonseekable semi-standard files
> > back to life. I guess, it is the best solution for now. And, also, XEN
> > guys are happy with this solution.
> > 
> > So what do you think about all this mess and may be it is possible to
> > have got better solution?
> 
> 	What the devil does that have to do with seeks, anyway?  Exact
> same problem will happen for blocking read() vs. another read() attempts
> on the same descriptor.  With perfectly accepted lseek() (which will also
> have to block, as per 2.9.7).

Yes, the problem here is because this particular file (/proc/xen/xenbus)
blocks the read() operation waiting for new events. Because of said
commit, now it also blocks write() operation used to send some request
(which would result in some response, so unblocking read() call). It
shouldn't be a normal file in the first place...

> 	Which file is that?  And what behaviour did you get on old kernels
> with it?  All reads block inside ->read() rather than sys_read()?

This is /proc/xen/xenbus. 

> 	Details, please.  Your strace doesn't show the relevant open(),
> so it's hard to tell what's really going on there in that regression.
> I agree that user-visible behaviour changes need to be dealt with; it's
> the nature of your fix I've a problem with.

Maybe its better to change the file type? Character device? Pipe?

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

Content of type "application/pgp-signature" skipped