linux-kernel - Re: general protection fault in relay_open

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 31 Jan 2019 19:46:57 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        syzbot <syzbot+16c3a70e1e9b29346c43@...kaller.appspotmail.com>,
        Eric Biggers <ebiggers@...gle.com>,
        Souptick Joarder <jrdr.linux@...il.com>,
        LKML <linux-kernel@...r.kernel.org>,
        David Rientjes <rientjes@...gle.com>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>
Subject: Re: general protection fault in relay_open_buf

On Fri, Feb 01, 2019 at 07:31:48AM +1300, Kees Cook wrote:
> On Thu, Jan 31, 2019 at 11:51 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> > Can you test the patch below?
> >
> > thanks,
> >
> > greg k-h
> >
> > --------------
> >
> > diff --git a/kernel/relay.c b/kernel/relay.c
> > index 04f248644e06..9e0f52375487 100644
> > --- a/kernel/relay.c
> > +++ b/kernel/relay.c
> > @@ -428,6 +428,8 @@ static struct dentry *relay_create_buf_file(struct rchan *chan,
> >         dentry = chan->cb->create_buf_file(tmpname, chan->parent,
> >                                            S_IRUSR, buf,
> >                                            &chan->is_global);
> > +       if (IS_ERR(dentry))
> > +               dentry = NULL;
> >
> >         kfree(tmpname);
> >
> > @@ -461,7 +463,7 @@ static struct rchan_buf *relay_open_buf(struct rchan *chan, unsigned int cpu)
> >                 dentry = chan->cb->create_buf_file(NULL, NULL,
> >                                                    S_IRUSR, buf,
> >                                                    &chan->is_global);
> > -               if (WARN_ON(dentry))
> > +               if (IS_ERR_OR_NULL(dentry))
> >                         goto free_buf;
> >         }
> >
> 
> Thanks! (Can we find other cases of this with static analysis?)

Probably.  I have over 100 patches to help clean up a lot of the debugfs
mess.  But it is very rare that someone actually tries to use the result
of a debugfs call as a "real" dentry, except to pass it back into
another debugfs call.  I "think" I have now caught all of those cases,
and if you can come up with some kind of rule for this, that would be
great.

But note, the create_buf_file() callback is the one that does the
debugfs call, so trying to figure out where that is coming from, what it
does, and what the dentry is later used for, spans lots of different
subsystems and files.  I don't think we have tools to do that, other
than grep :)

greg k-h