linux-kernel - Re: [PATCH v2] Input: uinput: Avoid Object-Already-Free with a global lock

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:   Tue, 23 Apr 2019 13:15:40 +0100
From:   Al Viro <viro@...iv.linux.org.uk>
To:     "dmitry.torokhov@...il.com" <dmitry.torokhov@...il.com>
Cc:     Mukesh Ojha <mojha@...eaurora.org>, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org, Gaurav Kohli <gkohli@...eaurora.org>,
        Peter Hutterer <peter.hutterer@...-t.net>,
        Martin Kepplinger <martink@...teo.de>,
        "Paul E. McKenney" <paulmck@...ux.ibm.com>
Subject: Re: [PATCH v2] Input: uinput: Avoid Object-Already-Free with a
 global lock

On Tue, Apr 23, 2019 at 12:06:12PM +0100, Al Viro wrote:
> On Tue, Apr 23, 2019 at 08:49:44AM +0000, dmitry.torokhov@...il.com wrote:
> > On Tue, Apr 23, 2019 at 12:51:13PM +0530, Mukesh Ojha wrote:
> 
> > > I have taken care this case from ioctl and release point of view.
> > > 
> > > Even if the release gets called first it will make the
> > > file->private_data=NULL.
> > > and further call to ioctl will not be a problem as the check is already
> > > there.
> > 
> > Al, do we have any protections in VFS layer from userspace hanging onto
> > a file descriptor and calling ioctl() on it even as another thread
> > calls close() on the same fd?
> > 
> > Should the issue be solved by individual drivers, or more careful
> > accounting for currently running operations is needed at VFS layer?
> 
> Neither.  An overlap of ->release() and ->ioctl() is possible only
> if you've got memory corruption somewhere.
> 
> close() overlapping ioctl() is certainly possible, and won't trigger
> that at all - sys_ioctl() holds onto reference to struct file, so
> its refcount won't reach zero until we are done with it.

I'd like to see a reproducer; _if_ we are really getting such overlaps,
we have something buggering struct file refcounts (e.g. stray fput()
eventually leading to dangling pointer in descriptor table) or an
outright memory corruption mangling descriptor table/kernel stack/
refcount/->private_data/etc.

Details, please - I tried to google some context for that, but hadn't
found anything useful ;-/