[PATCH RFC] smb: client: fix NULL pointer dereference in smb2_compound_op

Fredric Cover posted 1 patch 1 month, 2 weeks ago
fs/smb/client/smb2inode.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
[PATCH RFC] smb: client: fix NULL pointer dereference in smb2_compound_op
Posted by Fredric Cover 1 month, 2 weeks ago
From: Fredric Cover <fredric.cover.lkernel@gmail.com>

When a valid cfile is passed to smb2_compound_op, the code uses a
goto to jump to the 'after_open' label. This bypasses the
initialization of rqst[0] by SMB2_open_init().

However, the 'finished' cleanup block unconditionally attempts to
call SMB2_open_free() on rqst[0]. If SMB2_open_init() was skipped,
rqst[0].rq_iov is NULL, leading to a NULL pointer dereference.

Check for a valid rq_iov before attempting to free the request
to prevent the crash in handle-reuse scenarios.

Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com>
---
I am new to writing code for the Linux kernel.
Please be patient with me and feel free to point out any
obvious mistakes I made. 
I tested this patch in a user-space skeleton clone using ASan.
I only have access to a shallow clone and cannot find the 
original commit for the 'Fixes:' line.
---
 fs/smb/client/smb2inode.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
index c6dd282fc..aed96eeba 100644
--- a/fs/smb/client/smb2inode.c
+++ b/fs/smb/client/smb2inode.c
@@ -552,7 +552,13 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
 
 finished:
 	num_rqst = 0;
-	SMB2_open_free(&rqst[num_rqst++]);
+	/*
+	 * If cfile was passed, the open was skipped; check rq_iov
+	 * to ensure we don't try to dereference a NULL pointer.
+	 */
+	if (rqst[num_rqst].rq_iov)
+		SMB2_open_free(&rqst[num_rqst]);
+	num_rqst++;
 	if (rc == -EREMCHG) {
 		pr_warn_once("server share %s deleted\n", tcon->tree_name);
 		tcon->need_reconnect = true;
-- 
2.43.0
Re: [PATCH RFC] smb: client: fix NULL pointer dereference in smb2_compound_op
Posted by Fredric Cover 1 month, 2 weeks ago
(Apologies for the duplicate; adding the list to the thread for the archive)

Please disregard this patch. It adds a redundant check; there is no
NULL pointer dereference, and rqst is correctly zero-initialized using
kzalloc_obj().
Sorry for the confusion.


On Mon, Apr 27, 2026 at 7:52 PM Fredric Cover
<fredric.cover.lkernel@gmail.com> wrote:
>
> From: Fredric Cover <fredric.cover.lkernel@gmail.com>
>
> When a valid cfile is passed to smb2_compound_op, the code uses a
> goto to jump to the 'after_open' label. This bypasses the
> initialization of rqst[0] by SMB2_open_init().
>
> However, the 'finished' cleanup block unconditionally attempts to
> call SMB2_open_free() on rqst[0]. If SMB2_open_init() was skipped,
> rqst[0].rq_iov is NULL, leading to a NULL pointer dereference.
>
> Check for a valid rq_iov before attempting to free the request
> to prevent the crash in handle-reuse scenarios.
>
> Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com>
> ---
> I am new to writing code for the Linux kernel.
> Please be patient with me and feel free to point out any
> obvious mistakes I made.
> I tested this patch in a user-space skeleton clone using ASan.
> I only have access to a shallow clone and cannot find the
> original commit for the 'Fixes:' line.
> ---
>  fs/smb/client/smb2inode.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> index c6dd282fc..aed96eeba 100644
> --- a/fs/smb/client/smb2inode.c
> +++ b/fs/smb/client/smb2inode.c
> @@ -552,7 +552,13 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
>
>  finished:
>         num_rqst = 0;
> -       SMB2_open_free(&rqst[num_rqst++]);
> +       /*
> +        * If cfile was passed, the open was skipped; check rq_iov
> +        * to ensure we don't try to dereference a NULL pointer.
> +        */
> +       if (rqst[num_rqst].rq_iov)
> +               SMB2_open_free(&rqst[num_rqst]);
> +       num_rqst++;
>         if (rc == -EREMCHG) {
>                 pr_warn_once("server share %s deleted\n", tcon->tree_name);
>                 tcon->need_reconnect = true;
> --
> 2.43.0
>