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@google.com wrote:
On Mon, Jun 22, 2020 at 1:09 PM Christian Brauner christian.brauner@ubuntu.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@google.com
Forgot to include stable. The issue was introduced in 5.6, so fix needed in 5.7. Cc: stable@vger.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