linux-kernel - Re: [PATCH] binder: fix null deref of proc->context

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: <20200623052224.GC2252466@kroah.com>
Date:   Tue, 23 Jun 2020 07:22:24 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Todd Kjos <tkjos@...gle.com>
Cc:     Christian Brauner <christian.brauner@...ntu.com>,
        Christian Brauner <christian@...uner.io>,
        Arve Hjønnevåg <arve@...roid.com>,
        "open list:ANDROID DRIVERS" <devel@...verdev.osuosl.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Martijn Coenen <maco@...gle.com>,
        "Joel Fernandes (Google)" <joel@...lfernandes.org>,
        Android Kernel Team <kernel-team@...roid.com>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH] binder: fix null deref of proc->context

On Mon, Jun 22, 2020 at 01:59:04PM -0700, Todd Kjos wrote:
> On Mon, Jun 22, 2020 at 1:18 PM Todd Kjos <tkjos@...gle.com> wrote:
> >
> > On Mon, Jun 22, 2020 at 1:09 PM Christian Brauner
> > <christian.brauner@...ntu.com> wrote:
> > >
> > > On Mon, Jun 22, 2020 at 01:07:15PM -0700, Todd Kjos wrote:
> > > > The binder driver makes the assumption proc->context pointer is invariant after
> > > > initialization (as documented in the kerneldoc header for struct proc).
> > > > However, in commit f0fe2c0f050d ("binder: prevent UAF for binderfs devices II")
> > > > proc->context is set to NULL during binder_deferred_release().
> > > >
> > > > Another proc was in the middle of setting up a transaction to the dying
> > > > process and crashed on a NULL pointer deref on "context" which is a local
> > > > set to &proc->context:
> > > >
> > > >     new_ref->data.desc = (node == context->binder_context_mgr_node) ? 0 : 1;
> > > >
> > > > Here's the stack:
> > > >
> > > > [ 5237.855435] Call trace:
> > > > [ 5237.855441] binder_get_ref_for_node_olocked+0x100/0x2ec
> > > > [ 5237.855446] binder_inc_ref_for_node+0x140/0x280
> > > > [ 5237.855451] binder_translate_binder+0x1d0/0x388
> > > > [ 5237.855456] binder_transaction+0x2228/0x3730
> > > > [ 5237.855461] binder_thread_write+0x640/0x25bc
> > > > [ 5237.855466] binder_ioctl_write_read+0xb0/0x464
> > > > [ 5237.855471] binder_ioctl+0x30c/0x96c
> > > > [ 5237.855477] do_vfs_ioctl+0x3e0/0x700
> > > > [ 5237.855482] __arm64_sys_ioctl+0x78/0xa4
> > > > [ 5237.855488] el0_svc_common+0xb4/0x194
> > > > [ 5237.855493] el0_svc_handler+0x74/0x98
> > > > [ 5237.855497] el0_svc+0x8/0xc
> > > >
> > > > The fix is to move the kfree of the binder_device to binder_free_proc()
> > > > so the binder_device is freed when we know there are no references
> > > > remaining on the binder_proc.
> > > >
> > > > Fixes: f0fe2c0f050d ("binder: prevent UAF for binderfs devices II")
> > > > Signed-off-by: Todd Kjos <tkjos@...gle.com>
> >
> > Forgot to include stable. The issue was introduced in 5.6, so fix needed in 5.7.
> > Cc: stable@...r.kernel.org # 5.7
> 
> Turns out the patch with the issue was also backported to 5.4.y, so
> the fix is needed there too.

With the fixes tag in there and cc: stable, it will get to the proper
trees no matter how far back it was backported :)

thanks,

greg k-h