Messages in this thread | | | Date | Sat, 7 Oct 2023 19:07:40 +0800 | Subject | Re: [PATCH] binder: add mutex_lock for mmap and NULL when free | From | Kassey Li <> |
| |
On 2023/10/7 14:44, Greg KH wrote: > On Sat, Oct 07, 2023 at 11:40:46AM +0800, Kassey Li wrote: >> Enforce alloc->mutex in binder_alloc_mmap_handler when add >> the entry to list. >> >> Assign the freed pages/page_ptr to NULL to catch possible >> use after free with NULL pointer access. >> >> Signed-off-by: Kassey Li <quic_yingangl@quicinc.com> >> --- >> drivers/android/binder_alloc.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) > > What commit id does this fix?
there is no specific commit id this change going to fix.
it is a follow up for commit 19c987241ca1216a51118b2bd0185b8bc5081783 binder: separate out binder_alloc functions (mutex lock added for list access in alloc/free) f2517eb76f1f2f7f89761f9db2b202e89931738c android: binder: Add global lru shrinker to binder (set page->page_ptr = NULL;)
the background to raise this change that we are easy hit below crash in monkey test:
where a wrong end is passing to binder_update_page_range, thus calculate a weird index for page = &alloc->pages[index]
then causing the crash:
Unable to handle kernel paging request at virtual address 01ffff8724ade180
here this change will make sure the list access with mutex locked, and freed pointer to NULL to easy the debug for such kind of issues.
pc : list_lru_add+0x30/0x11c lr : list_lru_add+0x30/0x11c sp : ffffffc0374bb9c0 x29: ffffffc0374bb9c0 x28: ffffff890dc82000 x27: ffffff89df890000 x26: ffffffdf957a86f8 x25: 000ffffff0b72e0c x24: ffffffdf965a4208 x23: ffffffdf95585008 x22: ffffff89d6170000 x21: ffffffdf965a4208 x20: 01ffff8724ade180 x19: ffffff8022340580 x18: ffffffdf957bae60 x17: 00000000529c6ef0 x16: 00000000529c6ef0 x15: 00000000226956ae x14: 00000000e8903d35 x13: 00000000ebb92cf6 x12: ffffff89df890b50 x11: 00000000000000e0 x10: 0000000000000000 x9 : 0000000000000080 x8 : 00000000000000c0 x7 : 0000000000000000 x6 : ffffffdf93764970 x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000 x2 : 0000000000000001 x1 : ffffffdf94717c05 x0 : 0000000000000005 Call trace: list_lru_add+0x30/0x11c binder_update_page_range+0x154/0x994 binder_free_buf_locked+0x120/0x214 binder_alloc_free_buf+0xf4/0x118 binder_free_buf+0x284/0x390 binder_ioctl_write_read+0x1050/0x2d50 binder_ioctl+0x4f0/0x21c0 __arm64_sys_ioctl+0xa8/0xe4 invoke_syscall+0x58/0x114 el0_svc_common+0x94/0x118 do_el0_svc+0x2c/0xb8 el0_svc+0x30/0xb0 el0t_64_sync_handler+0x68/0xb4 el0t_64_sync+0x1a0/0x1a4
crash> list 0xFFFFFF890D063800 -s binder_buffer -x ffffff890d063800 struct binder_buffer { entry = { next = 0xffffff8a6c0fde00, prev = 0xffffff88e2e56ab0 }, rb_node = { __rb_parent_color = 0xffffff895461a811, rb_right = 0x0, rb_left = 0x0 }, free = 0x0, clear_on_free = 0x0, allow_user_free = 0x0, async_transaction = 0x0, oneway_spam_suspect = 0x0, debug_id = 0x5d16463, transaction = 0x0, target_node = 0xffffff898724b000, data_size = 0x68, offsets_size = 0x8, extra_buffers_size = 0x0, user_data = 0x7e63364000, pid = 0x4950 } ffffff8a6c0fde00 struct binder_buffer { entry = { next = 0xffffff895461a800, prev = 0xffffff890d063800 }, rb_node = { __rb_parent_color = 0xffffff895461a811, rb_right = 0x0, rb_left = 0xffffff89ebbcd610 }, free = 0x0, clear_on_free = 0x0, allow_user_free = 0x0, async_transaction = 0x0, oneway_spam_suspect = 0x0, debug_id = 0x5d16467, transaction = 0xffffff8a0435a300, target_node = 0xffffff8978905b00, data_size = 0x84, offsets_size = 0x0, extra_buffers_size = 0x0, user_data = 0xffffff89d6171a00, // bad address pid = 0x5584 }
> > thanks, > > greg k-h
| |