linux-kernel - Re: [PATCH] devpts: Fix NULL pointer dereference in dcache

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: <20191004152526.adgg3a7u7jylfk4a@wittgenstein>
Date:   Fri, 4 Oct 2019 17:25:28 +0200
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     Al Viro <viro@...iv.linux.org.uk>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, Will Deacon <will@...nel.org>,
        Kate Stewart <kstewart@...uxfoundation.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Amir Goldstein <amir73il@...il.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Varad Gautam <vrd@...zon.de>, stable@...r.kernel.org,
        Jan Glauber <jglauber@...vell.com>
Subject: Re: [PATCH] devpts: Fix NULL pointer dereference in dcache_readdir()

On Fri, Oct 04, 2019 at 04:10:58PM +0100, Al Viro wrote:
> On Fri, Oct 04, 2019 at 04:33:02PM +0200, Christian Brauner wrote:
> > On Fri, Oct 04, 2019 at 03:27:48PM +0100, Al Viro wrote:
> > > On Fri, Oct 04, 2019 at 04:05:03PM +0200, Christian Brauner wrote:
> > > > From: Will Deacon <will@...nel.org>
> > > > 
> > > > Closing /dev/pts/ptmx removes the corresponding pty under /dev/pts/
> > > > without synchronizing against concurrent path walkers. This can lead to
> > > > 'dcache_readdir()' tripping over a 'struct dentry' with a NULL 'd_inode'
> > > > field:
> > > 
> > > FWIW, vfs.git#fixes (or #next.dcache) ought to deal with that one.
> > 
> > Is it feasible to backport your changes? Or do we want to merge the one
> > here first and backport?
> 
> I'm not sure.  The whole pile is backportable, all right (and the first commit

Ok. So here's what I propose: we'll merge this one as it seems an
obvious fix to the problem and can easily be backported to stable
kernels.
Then you'll land your generic workaround alleviating callers from
holding inode_lock(). Then I'll send a patch to remove the inode_lock()
from devpts for master.
If we see that your fix is fine to backport and has no performance
impacts that you find unacceptable we backport it.

> alone should take care of devpts problem).  However, there's a performance
> regression on some loads; it *is* possible to get the thing reasonably lockless
> without fucking it up (as the original conversion had been).  Still not
> in the series, since cifs (ab)use of dcache_readdir() needs to be clarified
> to figure out the right way to do it.  Asked CIFS folks, got no reaction
> whatsoever, will ask again...
> 
> 	Al, mostly back after flu, digging through the piles of mail

Ugh, flu season...

Christian