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 <[email protected]>
---
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;
}
--
2.18.0
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 <[email protected]>
> ---
> 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 <[email protected]>
On Fri, 2023-07-21 at 06:34 -0400, Jeff Layton wrote:
>
> 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.
>
Nevermind. I see how this could happen, and have gone ahead and merged
the patch. It should make v6.6.
Cheers,
--
Jeff Layton <[email protected]>
On Fri, 2023-07-21 at 08:11 -0400, Jeff Layton wrote:
>
> Nevermind. I see how this could happen, and have gone ahead and
> merged
> the patch. It should make v6.6.
>
> Cheers,
> --
> Jeff Layton <[email protected]>
Dear Jeff,
Really thanks for your reply. we'd want to know could the patch we
delivered, which moves ftrace function ahead the removing APIs, be the
finial patch and will submit in k6.6. If yes, we would try to ask
google owner to patch back the patch in k6.1. Or is there any advice to
show us how to fix it. Thanks again.
Best Regards,
Will Shiu