On Fri, 2024-06-14 at 06:31 +0000, Light Hsieh (謝明燈) wrote: > Hi, Jeff > > We still get use-after-free issue on trace_posix_lock_inode() within posix_lock_inodes(). > Do you think invoke trace_posix_lock_inode() just before spin_unlock(&ctx->flc_lock) is better? > (cc'ing linux-fsdevel and the other file locking maintainers) Thanks for the bug report! Note that the UAF was detected in the context of pid 26303, but the object was allocated and freed by pid 26304. So, this is a race between threads. I suspect that what's happening is that the "request" pointer is being overwritten by one of the objects on the list and then that lock is being freed after we release the spinlock but before the tracepoint happens. Moving the tracepoint inside the spinlock does sound like what we need to do. Care to propose a patch? Thanks, Jeff > [36705.493785] [T626354] BackgroundHandl: audit: audit_lost=637997 audit_rate_limit=5 audit_backlog_limit=64 > [36705.493799] [T626354] BackgroundHandl: audit: rate limit exceeded > [36705.558088] [T226303] BackgroundHandl: ================================================================== > [36705.558104] [T226303] BackgroundHandl: BUG: KASAN: slab-use-after-free in trace_event_raw_event_filelock_lock+0x7c/0x128 > [36705.558123] [T226303] BackgroundHandl: Read at addr fbffff80d9ea9dc0 by task BackgroundHandl/26303 > [36705.558129] [T226303] BackgroundHandl: Pointer tag: [fb], memory tag: [fe] > [36705.558132] [T226303] BackgroundHandl: > [36705.558136] [T226303] BackgroundHandl: CPU: 2 PID: 26303 Comm: BackgroundHandl Tainted: G W OE 6.6.29-android15-5-gaace8ad45e01 #1 > [36705.558142] [T226303] BackgroundHandl: Hardware name: MT6991(ENG) (DT) > [36705.558145] [T226303] BackgroundHandl: Call trace: > [36705.558148] [T226303] BackgroundHandl: dump_backtrace+0xec/0x138 > [36705.558155] [T226303] BackgroundHandl: show_stack+0x18/0x24 > [36705.558159] [T226303] BackgroundHandl: dump_stack_lvl+0x50/0x6c > [36705.558165] [T226303] BackgroundHandl: print_report+0x1b0/0x714 > [36705.558175] [T226303] BackgroundHandl: kasan_report+0xc4/0x124 > [36705.558179] [T226303] BackgroundHandl: __do_kernel_fault+0xbc/0x2d4 > [36705.558191] [T226303] BackgroundHandl: do_bad_area+0x30/0xdc > [36705.558196] [T226303] BackgroundHandl: do_tag_check_fault+0x20/0x34 > [36705.558200] [T226303] BackgroundHandl: do_mem_abort+0x58/0x118 > [36705.558205] [T226303] BackgroundHandl: el1_abort+0x3c/0x5c > [36705.558209] [T226303] BackgroundHandl: el1h_64_sync_handler+0x54/0x90 > [36705.558213] [T226303] BackgroundHandl: el1h_64_sync+0x68/0x6c > [36705.558217] [T226303] BackgroundHandl: trace_event_raw_event_filelock_lock+0x7c/0x128 > [36705.558222] [T226303] BackgroundHandl: posix_lock_inode+0xe34/0xe94 > [36705.558228] [T226303] BackgroundHandl: do_lock_file_wait+0xc4/0x1a4 > [36705.558234] [T226303] BackgroundHandl: fcntl_setlk+0x2dc/0x448 > [36705.558239] [T226303] BackgroundHandl: do_fcntl+0x90/0x570 > [36705.558251] [T226303] BackgroundHandl: __arm64_sys_fcntl+0x7c/0xc8 > [36705.558256] [T226303] BackgroundHandl: invoke_syscall+0x58/0x114 > [36705.558269] [T226303] BackgroundHandl: el0_svc_common+0x80/0xe0 > [36705.558274] [T226303] BackgroundHandl: do_el0_svc+0x1c/0x28 > [36705.558279] [T226303] BackgroundHandl: el0_svc+0x38/0x68 > [36705.558283] [T226303] BackgroundHandl: el0t_64_sync_handler+0x68/0xbc > [36705.558287] [T226303] BackgroundHandl: el0t_64_sync+0x1a8/0x1ac > [36705.558290] [T226303] BackgroundHandl: > [36705.558292] [T226303] BackgroundHandl: Allocated by task 26304: > [36705.558296] [T226303] BackgroundHandl: kasan_save_stack+0x40/0x70 > [36705.558300] [T226303] BackgroundHandl: save_stack_info+0x34/0x128 > [36705.558306] [T226303] BackgroundHandl: kasan_save_alloc_info+0x14/0x20 > [36705.558310] [T226303] BackgroundHandl: __kasan_slab_alloc+0x168/0x174 > [36705.558315] [T226303] BackgroundHandl: slab_post_alloc_hook+0x88/0x3a4 > [36705.558331] [T226303] BackgroundHandl: kmem_cache_alloc+0x18c/0x2c8 > [36705.558336] [T226303] BackgroundHandl: posix_lock_inode+0xb4/0xe94 > [36705.558341] [T226303] BackgroundHandl: do_lock_file_wait+0xc4/0x1a4 > [36705.558346] [T226303] BackgroundHandl: fcntl_setlk+0x2dc/0x448 > [36705.558350] [T226303] BackgroundHandl: do_fcntl+0x90/0x570 > [36705.558355] [T226303] BackgroundHandl: __arm64_sys_fcntl+0x7c/0xc8 > [36705.558360] [T226303] BackgroundHandl: invoke_syscall+0x58/0x114 > [36705.558365] [T226303] BackgroundHandl: el0_svc_common+0x80/0xe0 > [36705.558370] [T226303] BackgroundHandl: do_el0_svc+0x1c/0x28 > [36705.558375] [T226303] BackgroundHandl: el0_svc+0x38/0x68 > [36705.558378] [T226303] BackgroundHandl: el0t_64_sync_handler+0x68/0xbc > [36705.558382] [T226303] BackgroundHandl: el0t_64_sync+0x1a8/0x1ac > [36705.558385] [T226303] BackgroundHandl: > [36705.558387] [T226303] BackgroundHandl: Freed by task 26304: > [36705.558391] [T226303] BackgroundHandl: kasan_save_stack+0x40/0x70 > [36705.558395] [T226303] BackgroundHandl: save_stack_info+0x34/0x128 > [36705.558399] [T226303] BackgroundHandl: kasan_save_free_info+0x18/0x28 > [36705.558403] [T226303] BackgroundHandl: ____kasan_slab_free+0x254/0x25c > [36705.558407] [T226303] BackgroundHandl: __kasan_slab_free+0x10/0x20 > [36705.558411] [T226303] BackgroundHandl: slab_free_freelist_hook+0x174/0x1e0 > [36705.558415] [T226303] BackgroundHandl: kmem_cache_free+0xc4/0x348 > [36705.558420] [T226303] BackgroundHandl: locks_dispose_list+0x3c/0x164 > [36705.558425] [T226303] BackgroundHandl: posix_lock_inode+0xb78/0xe94 > [36705.558429] [T226303] BackgroundHandl: do_lock_file_wait+0xc4/0x1a4 > [36705.558434] [T226303] BackgroundHandl: fcntl_setlk+0x2dc/0x448 > [36705.558439] [T226303] BackgroundHandl: do_fcntl+0x90/0x570 > [36705.558444] [T226303] BackgroundHandl: __arm64_sys_fcntl+0x7c/0xc8 > [36705.558448] [T226303] BackgroundHandl: invoke_syscall+0x58/0x114 > [36705.558454] [T226303] BackgroundHandl: el0_svc_common+0x80/0xe0 > [36705.558459] [T226303] BackgroundHandl: do_el0_svc+0x1c/0x28 > [36705.558464] [T226303] BackgroundHandl: el0_svc+0x38/0x68 > [36705.558468] [T226303] BackgroundHandl: el0t_64_sync_handler+0x68/0xbc > [36705.558472] [T226303] BackgroundHandl: el0t_64_sync+0x1a8/0x1ac > [36705.558475] [T226303] BackgroundHandl: > [36705.558477] [T226303] BackgroundHandl: The buggy address belongs to the object at ffffff80d9ea9dc0 > which belongs to the cache file_lock_cache of size 216 > [36705.558481] [T226303] BackgroundHandl: The buggy address is located 0 bytes inside of > 216-byte region [ffffff80d9ea9dc0, ffffff80d9ea9e98) > [36705.558485] [T226303] BackgroundHandl: > [36705.558487] [T226303] BackgroundHandl: The buggy address belongs to the physical page: > [36705.558491] [T226303] BackgroundHandl: page:000000002f0b235f refcount:1 mapcount:0 mapping:0000000000000000 index:0xf6ffff80d9ea9b20 pfn:0x159ea8 > [36705.558495] [T226303] BackgroundHandl: head:000000002f0b235f order:1 entire_mapcount:0 nr_pages_mapped:0 pincount:0 > [36705.558499] [T226303] BackgroundHandl: flags: 0x4000000000000840(slab|head|zone=1|kasantag=0x0) > [36705.558505] [T226303] BackgroundHandl: page_type: 0xffffffff() > [36705.558509] [T226303] BackgroundHandl: raw: 4000000000000840 f8ffff8012e57b00 fffffffe06a4fb00 0000000000000004 > [36705.558513] [T226303] BackgroundHandl: raw: f6ffff80d9ea9b20 0000000080240021 00000001ffffffff 0000000000000000 > [36705.558516] [T226303] BackgroundHandl: page dumped because: kasan: bad access detected > [36705.558519] [T226303] BackgroundHandl: page_owner tracks the page as allocated > [36705.558522] [T226303] BackgroundHandl: page last allocated via order 1, migratetype Unmovable, gfp_mask 0x1d20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC|__GFP_HARDWALL), pid 23092, tgid 23025 (elastic_NPS), ts 15865705548286, free_ts 15865538036285 > [36705.558529] [T226303] BackgroundHandl: post_alloc_hook+0x18c/0x194 > [36705.558542] [T226303] BackgroundHandl: prep_new_page+0x28/0x13c > [36705.558548] [T226303] BackgroundHandl: get_page_from_freelist+0x1928/0x198c > [36705.558553] [T226303] BackgroundHandl: __alloc_pages+0x158/0x310 > [36705.558558] [T226303] BackgroundHandl: alloc_slab_page+0x8c/0x178 > [36705.558562] [T226303] BackgroundHandl: new_slab+0xa4/0x340 > [36705.558565] [T226303] BackgroundHandl: ___slab_alloc+0x648/0x98c > [36705.558569] [T226303] BackgroundHandl: __slab_alloc+0x6c/0xac > [36705.558575] [T226303] BackgroundHandl: kmem_cache_alloc+0x1e8/0x2c8 > [36705.558580] [T226303] BackgroundHandl: posix_lock_inode+0x108/0xe94 > [36705.558584] [T226303] BackgroundHandl: do_lock_file_wait+0xc4/0x1a4 > [36705.558589] [T226303] BackgroundHandl: fcntl_setlk+0x2dc/0x448 > [36705.558594] [T226303] BackgroundHandl: do_fcntl+0x90/0x570 > [36705.558599] [T226303] BackgroundHandl: __arm64_sys_fcntl+0x7c/0xc8 > [36705.558603] [T226303] BackgroundHandl: invoke_syscall+0x58/0x114 > [36705.558609] [T226303] BackgroundHandl: el0_svc_common+0x80/0xe0 > [36705.558614] [T226303] BackgroundHandl: page last free pid 23092 tgid 23025 stack trace: > [36705.558617] [T226303] BackgroundHandl: free_unref_page_prepare+0x2e8/0x310 > [36705.558622] [T226303] BackgroundHandl: free_unref_page_list+0x90/0x374 > [36705.558627] [T226303] BackgroundHandl: release_pages+0x44c/0x488 > [36705.558642] [T226303] BackgroundHandl: free_pages_and_swap_cache+0x58/0x70 > [36705.558648] [T226303] BackgroundHandl: tlb_flush_mmu+0xa0/0x144 > [36705.558662] [T226303] BackgroundHandl: tlb_finish_mmu+0x48/0xb0 > [36705.558667] [T226303] BackgroundHandl: zap_page_range_single+0x168/0x1a0 > [36705.558671] [T226303] BackgroundHandl: do_madvise+0x2f4/0xf00 > [36705.558675] [T226303] BackgroundHandl: __arm64_sys_madvise+0x20/0x34 > [36705.558679] [T226303] BackgroundHandl: invoke_syscall+0x58/0x114 > [36705.558684] [T226303] BackgroundHandl: el0_svc_common+0x80/0xe0 > [36705.558689] [T226303] BackgroundHandl: do_el0_svc+0x1c/0x28 > [36705.558694] [T226303] BackgroundHandl: el0_svc+0x38/0x68 > [36705.558698] [T226303] BackgroundHandl: el0t_64_sync_handler+0x68/0xbc > [36705.558701] [T226303] BackgroundHandl: el0t_64_sync+0x1a8/0x1ac > [36705.558705] [T226303] BackgroundHandl: > [36705.558708] [T226303] BackgroundHandl: Memory state around the buggy address: > [36705.558711] [T226303] BackgroundHandl: ffffff80d9ea9b00: f2 f2 fe fe fe fe fe fe fe fe fe fe fe fe fe fe > [36705.558714] [T226303] BackgroundHandl: ffffff80d9ea9c00: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f5 f5 > [36705.558718] [T226303] BackgroundHandl: >ffffff80d9ea9d00: f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 f5 fe fe fe fe > [36705.558721] [T226303] BackgroundHandl: ^ > [36705.558724] [T226303] BackgroundHandl: ffffff80d9ea9e00: fe fe fe fe fe fe fe fe fe fe fa fa fa fa fa fa > [36705.558728] [T226303] BackgroundHandl: ffffff80d9ea9f00: fa fa fa fa fa fa fa fa fe fe fe fe fe fe fe fe > [36705.558738] [T226303] BackgroundHandl: ================================================================== > > > > -----Original Message----- > From: Jeff Layton <jlayton@xxxxxxxxxx> > Sent: Friday, July 21, 2023 6:35 PM > To: Will Shiu (許恭瑜) <Will.Shiu@xxxxxxxxxxxx>; Chuck Lever <chuck.lever@xxxxxxxxxx>; Alexander Viro <viro@xxxxxxxxxxxxxxxxxx>; Christian Brauner <brauner@xxxxxxxxxx>; Matthias Brugger <matthias.bgg@xxxxxxxxx>; AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx>; linux-fsdevel@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-mediatek@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH] Fix BUG: KASAN: use-after-free in trace_event_raw_event_filelock_lock > > On Fri, 2023-07-21 at 13:19 +0800, Will Shiu wrote: > > As following backtrace, the struct file_lock request , in > > posix_lock_inode is free before ftrace function using. > > Replace the ftrace function ahead free flow could fix the > > use-after-free issue. > > > > [name:report&]=============================================== > > BUG:KASAN: use-after-free in > > trace_event_raw_event_filelock_lock+0x80/0x12c > > [name:report&]Read at addr f6ffff8025622620 by task NativeThread/16753 > > [name:report_hw_tags&]Pointer tag: [f6], memory tag: [fe] > > [name:report&] > > BT: > > Hardware name: MT6897 (DT) > > Call trace: > > dump_backtrace+0xf8/0x148 > > show_stack+0x18/0x24 > > dump_stack_lvl+0x60/0x7c > > print_report+0x2c8/0xa08 > > kasan_report+0xb0/0x120 > > __do_kernel_fault+0xc8/0x248 > > do_bad_area+0x30/0xdc > > do_tag_check_fault+0x1c/0x30 > > do_mem_abort+0x58/0xbc > > el1_abort+0x3c/0x5c > > el1h_64_sync_handler+0x54/0x90 > > el1h_64_sync+0x68/0x6c > > trace_event_raw_event_filelock_lock+0x80/0x12c > > posix_lock_inode+0xd0c/0xd60 > > do_lock_file_wait+0xb8/0x190 > > fcntl_setlk+0x2d8/0x440 > > ... > > [name:report&] > > [name:report&]Allocated by task 16752: > > ... > > slab_post_alloc_hook+0x74/0x340 > > kmem_cache_alloc+0x1b0/0x2f0 > > posix_lock_inode+0xb0/0xd60 > > ... > > [name:report&] > > [name:report&]Freed by task 16752: > > ... > > kmem_cache_free+0x274/0x5b0 > > locks_dispose_list+0x3c/0x148 > > posix_lock_inode+0xc40/0xd60 > > do_lock_file_wait+0xb8/0x190 > > fcntl_setlk+0x2d8/0x440 > > do_fcntl+0x150/0xc18 > > ... > > > > Signed-off-by: Will Shiu <Will.Shiu@xxxxxxxxxxxx> > > --- > > fs/locks.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/locks.c b/fs/locks.c > > index df8b26a42524..a552bdb6badc 100644 > > --- a/fs/locks.c > > +++ b/fs/locks.c > > @@ -1301,6 +1301,7 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, > > out: > > spin_unlock(&ctx->flc_lock); > > percpu_up_read(&file_rwsem); > > + trace_posix_lock_inode(inode, request, error); > > /* > > * Free any unused locks. > > */ > > @@ -1309,7 +1310,6 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request, > > if (new_fl2) > > locks_free_lock(new_fl2); > > locks_dispose_list(&dispose); > > - trace_posix_lock_inode(inode, request, error); > > > > return error; > > } > > Could you send along the entire KASAN log message? I'm not sure I see how this is being tripped. The lock we're passing in here is "request" > and that shouldn't be freed since it's allocated and owned by the caller. > > -- > Jeff Layton <jlayton@xxxxxxxxxx> -- Jeff Layton <jlayton@xxxxxxxxxx>
