Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d50c6c34035f1a0b143507d9ab9fcf0d27a5dc86.camel@kernel.org>
Subject: Re: [PATCH] Fix BUG: KASAN: use-after-free in
 trace_event_raw_event_filelock_lock
From:   Jeff Layton <jlayton@kernel.org>
To:     Will Shiu <Will.Shiu@mediatek.com>,
        Chuck Lever <chuck.lever@oracle.com>,
        Alexander Viro <viro@zeniv.linux.org.uk>,
        Christian Brauner <brauner@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@collabora.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-mediatek@lists.infradead.org
Date:   Fri, 21 Jul 2023 06:34:40 -0400
In-Reply-To: <20230721051904.9317-1-Will.Shiu@mediatek.com>
References: <20230721051904.9317-1-Will.Shiu@mediatek.com>
Content-Type: text/plain; charset="ISO-8859-15"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Fri, 2023-07-21 at 13:19 +0800, Will Shiu wrote:
> As following backtrace, the struct file_lock request , in posix_lock_inod=
e
> is free before ftrace function using.
> Replace the ftrace function ahead free flow could fix the use-after-free
> issue.
>=20
> [name:report&]=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> BUG:KASAN: use-after-free in trace_event_raw_event_filelock_lock+0x80/0x1=
2c
> [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
> ...
>=20
> Signed-off-by: Will Shiu <Will.Shiu@mediatek.com>
> ---
>  fs/locks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>=20
> 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, st=
ruct 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, st=
ruct file_lock *request,
>  	if (new_fl2)
>  		locks_free_lock(new_fl2);
>  	locks_dispose_list(&dispose);
> -	trace_posix_lock_inode(inode, request, error);
> =20
>  	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.

--=20
Jeff Layton <jlayton@kernel.org>