diff mbox series

filelock: fix potential use-after-free in posix_lock_inode

Message ID 20240702-filelock-6-10-v1-1-96e766aadc98@kernel.org (mailing list archive)
State New, archived
Headers show
Series filelock: fix potential use-after-free in posix_lock_inode | expand

Commit Message

Jeff Layton July 2, 2024, 10:44 p.m. UTC
Light Hsieh reported a KASAN UAF warning in trace_posix_lock_inode().
The request pointer had been changed earlier to point to a lock entry
that was added to the inode's list. However, before the tracepoint could
fire, another task raced in and freed that lock.

Fix this by moving the tracepoint inside the spinlock, which should
ensure that this doesn't happen.

Fixes: 74f6f5912693 ("locks: fix KASAN: use-after-free in trace_event_raw_event_filelock_lock")
Link: https://lore.kernel.org/linux-fsdevel/724ffb0a2962e912ea62bb0515deadf39c325112.camel@kernel.org/
Reported-by: Light Hsieh (謝明燈) <Light.Hsieh@mediatek.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/locks.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: e9d22f7a6655941fc8b2b942ed354ec780936b3e
change-id: 20240702-filelock-6-10-3fa00ca4fe53

Best regards,

Comments

Alexander Aring July 3, 2024, 1:19 a.m. UTC | #1
Hi,

On Tue, Jul 2, 2024 at 6:45 PM Jeff Layton <jlayton@kernel.org> wrote:
>
> Light Hsieh reported a KASAN UAF warning in trace_posix_lock_inode().
> The request pointer had been changed earlier to point to a lock entry
> that was added to the inode's list. However, before the tracepoint could
> fire, another task raced in and freed that lock.
>
> Fix this by moving the tracepoint inside the spinlock, which should
> ensure that this doesn't happen.
>

makes sense to me. Thanks.

Reviewed-by: Alexander Aring <aahringo@redhat.com>

- Alex
Chuck Lever July 3, 2024, 1:53 p.m. UTC | #2
On Tue, Jul 02, 2024 at 06:44:48PM -0400, Jeff Layton wrote:
> Light Hsieh reported a KASAN UAF warning in trace_posix_lock_inode().
> The request pointer had been changed earlier to point to a lock entry
> that was added to the inode's list. However, before the tracepoint could
> fire, another task raced in and freed that lock.
> 
> Fix this by moving the tracepoint inside the spinlock, which should
> ensure that this doesn't happen.
> 
> Fixes: 74f6f5912693 ("locks: fix KASAN: use-after-free in trace_event_raw_event_filelock_lock")
> Link: https://lore.kernel.org/linux-fsdevel/724ffb0a2962e912ea62bb0515deadf39c325112.camel@kernel.org/
> Reported-by: Light Hsieh (謝明燈) <Light.Hsieh@mediatek.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/locks.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/locks.c b/fs/locks.c
> index c360d1992d21..bdd94c32256f 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -1367,9 +1367,9 @@ static int posix_lock_inode(struct inode *inode, struct file_lock *request,
>  		locks_wake_up_blocks(&left->c);
>  	}
>   out:
> +	trace_posix_lock_inode(inode, request, error);
>  	spin_unlock(&ctx->flc_lock);
>  	percpu_up_read(&file_rwsem);
> -	trace_posix_lock_inode(inode, request, error);
>  	/*
>  	 * Free any unused locks.
>  	 */
> 
> ---
> base-commit: e9d22f7a6655941fc8b2b942ed354ec780936b3e
> change-id: 20240702-filelock-6-10-3fa00ca4fe53
> 
> Best regards,
> -- 
> Jeff Layton <jlayton@kernel.org>
> 

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>
diff mbox series

Patch

diff --git a/fs/locks.c b/fs/locks.c
index c360d1992d21..bdd94c32256f 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1367,9 +1367,9 @@  static int posix_lock_inode(struct inode *inode, struct file_lock *request,
 		locks_wake_up_blocks(&left->c);
 	}
  out:
+	trace_posix_lock_inode(inode, request, error);
 	spin_unlock(&ctx->flc_lock);
 	percpu_up_read(&file_rwsem);
-	trace_posix_lock_inode(inode, request, error);
 	/*
 	 * Free any unused locks.
 	 */