lkml.org 
[lkml]   [2019]   [Oct]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4.9 30/47] ANDROID: binder: remove waitqueue when thread exits.
    On Mon, Oct 07, 2019 at 11:33:53AM +0200, Martijn Coenen wrote:
    > On Sun, Oct 6, 2019 at 7:23 PM Greg Kroah-Hartman
    > <gregkh@linuxfoundation.org> wrote:
    > >
    > > From: Martijn Coenen <maco@android.com>
    > >
    > > commit f5cb779ba16334b45ba8946d6bfa6d9834d1527f upstream.
    > >
    > > binder_poll() passes the thread->wait waitqueue that
    > > can be slept on for work. When a thread that uses
    > > epoll explicitly exits using BINDER_THREAD_EXIT,
    > > the waitqueue is freed, but it is never removed
    > > from the corresponding epoll data structure. When
    > > the process subsequently exits, the epoll cleanup
    > > code tries to access the waitlist, which results in
    > > a use-after-free.
    > >
    > > Prevent this by using POLLFREE when the thread exits.
    > >
    > > Signed-off-by: Martijn Coenen <maco@android.com>
    > > Reported-by: syzbot <syzkaller@googlegroups.com>
    > > Cc: stable <stable@vger.kernel.org> # 4.14
    > > [backport BINDER_LOOPER_STATE_POLL logic as well]
    > > Signed-off-by: Mattias Nissler <mnissler@chromium.org>
    > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    > > ---
    > > drivers/android/binder.c | 17 ++++++++++++++++-
    > > 1 file changed, 16 insertions(+), 1 deletion(-)
    > >
    > > --- a/drivers/android/binder.c
    > > +++ b/drivers/android/binder.c
    > > @@ -334,7 +334,8 @@ enum {
    > > BINDER_LOOPER_STATE_EXITED = 0x04,
    > > BINDER_LOOPER_STATE_INVALID = 0x08,
    > > BINDER_LOOPER_STATE_WAITING = 0x10,
    > > - BINDER_LOOPER_STATE_NEED_RETURN = 0x20
    > > + BINDER_LOOPER_STATE_NEED_RETURN = 0x20,
    > > + BINDER_LOOPER_STATE_POLL = 0x40,
    > > };
    > >
    > > struct binder_thread {
    > > @@ -2628,6 +2629,18 @@ static int binder_free_thread(struct bin
    > > } else
    > > BUG();
    > > }
    > > +
    > > + /*
    > > + * If this thread used poll, make sure we remove the waitqueue
    > > + * from any epoll data structures holding it with POLLFREE.
    > > + * waitqueue_active() is safe to use here because we're holding
    > > + * the inner lock.
    >
    > This should be "global lock" in 4.9 and 4.4 :)

    I'll go update the comment now, thanks!

    > Otherwise LGTM, thanks!

    thanks for the review.

    greg k-h

    \
     
     \ /
      Last update: 2019-10-07 11:39    [W:2.911 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site