[PATCH] Fix BUG: KASAN: use-after-free in trace_event_raw_event_filelock_lock

[PATCH] Fix BUG: KASAN: use-after-free in trace_event_raw_event_filelock_lock

[Will Shiu]

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@mediatek.com>
---
 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Fix BUG: KASAN: use-after-free in trace_event_raw_event_filelock_lock

[Jeff Layton]

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@mediatek.com>
> ---
>  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@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Fix BUG: KASAN: use-after-free in trace_event_raw_event_filelock_lock

[Jeff Layton]

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 <jlayton@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] Fix BUG: KASAN: use-after-free in trace_event_raw_event_filelock_lock

[Will Shiu (許恭瑜)]

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 <jlayton@kernel.org>

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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel