All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Aurélien Aptel" <aaptel@suse.com>
To: Dan Carpenter <dan.carpenter@oracle.com>,
	Steve French <sfrench@samba.org>,
	Pavel Shilovsky <pshilov@microsoft.com>
Cc: linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [patch] CIFS: set *resp_buf_type to NO_BUFFER on error
Date: Tue, 07 Feb 2017 15:33:21 +0000	[thread overview]
Message-ID: <mpsmvdyhv7i.fsf@aaptelpc.suse.de> (raw)
In-Reply-To: <20170207131841.GC31552@mwanda>

Dan Carpenter <dan.carpenter@oracle.com> writes:
> We recently shuffled this code around and introduced a new error path
> before *resp_buf_type gets initialized.  It creates uninitialized
> variable bugs in the callers.
>
>     fs/cifs/smb2pdu.c:579 SMB2_negotiate()
>     error: uninitialized symbol 'resp_buftype'.
>
> Fixes: 738f9de5cdb9 ("CIFS: Send RFC1001 length in a separate iov")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 526f0533cb4e..8fa5e058fb15 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -807,6 +807,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  	struct kvec *new_iov;
>  	int rc;
>  
> +	*resp_buf_type = CIFS_NO_BUFFER; /* no response buf yet */
> +
>  	new_iov = kmalloc(sizeof(struct kvec) * (n_vec + 1), GFP_KERNEL);
>  	if (!new_iov)
>  		return -ENOMEM;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

LGTM. To be a bit more explicit:

resp_buf_type is an output parameter of the SendReceive2 function and in
case the kmalloc failed the function could return to the caller with
this parameter left uninitialized.

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

WARNING: multiple messages have this Message-ID (diff)
From: "Aurélien Aptel" <aaptel@suse.com>
To: Dan Carpenter <dan.carpenter@oracle.com>,
	Steve French <sfrench@samba.org>,
	Pavel Shilovsky <pshilov@microsoft.com>
Cc: linux-cifs@vger.kernel.org, samba-technical@lists.samba.org,
	kernel-janitors@vger.kernel.org
Subject: Re: [patch] CIFS: set *resp_buf_type to NO_BUFFER on error
Date: Tue, 07 Feb 2017 16:33:21 +0100	[thread overview]
Message-ID: <mpsmvdyhv7i.fsf@aaptelpc.suse.de> (raw)
In-Reply-To: <20170207131841.GC31552@mwanda>

Dan Carpenter <dan.carpenter@oracle.com> writes:
> We recently shuffled this code around and introduced a new error path
> before *resp_buf_type gets initialized.  It creates uninitialized
> variable bugs in the callers.
>
>     fs/cifs/smb2pdu.c:579 SMB2_negotiate()
>     error: uninitialized symbol 'resp_buftype'.
>
> Fixes: 738f9de5cdb9 ("CIFS: Send RFC1001 length in a separate iov")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index 526f0533cb4e..8fa5e058fb15 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -807,6 +807,8 @@ SendReceive2(const unsigned int xid, struct cifs_ses *ses,
>  	struct kvec *new_iov;
>  	int rc;
>  
> +	*resp_buf_type = CIFS_NO_BUFFER; /* no response buf yet */
> +
>  	new_iov = kmalloc(sizeof(struct kvec) * (n_vec + 1), GFP_KERNEL);
>  	if (!new_iov)
>  		return -ENOMEM;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

LGTM. To be a bit more explicit:

resp_buf_type is an output parameter of the SendReceive2 function and in
case the kmalloc failed the function could return to the caller with
this parameter left uninitialized.

Reviewed-by: Aurelien Aptel <aaptel@suse.com>

-- 
Aurélien Aptel / SUSE Labs Samba Team
GPG: 1839 CB5F 9F5B FB9B AA97  8C99 03C8 A49B 521B D5D3
SUSE Linux GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)

  reply	other threads:[~2017-02-07 15:33 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-07 13:18 [patch] CIFS: set *resp_buf_type to NO_BUFFER on error Dan Carpenter
2017-02-07 13:18 ` Dan Carpenter
2017-02-07 15:33 ` Aurélien Aptel [this message]
2017-02-07 15:33   ` Aurélien Aptel
2017-02-08  1:00 ` Pavel Shilovsky
2017-02-08  1:00   ` Pavel Shilovsky
2018-04-22 15:30 ` Steve French
2018-04-23 11:54 ` Dan Carpenter
2018-04-23 15:17 ` Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mpsmvdyhv7i.fsf@aaptelpc.suse.de \
    --to=aaptel@suse.com \
    --cc=dan.carpenter@oracle.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pshilov@microsoft.com \
    --cc=samba-technical@lists.samba.org \
    --cc=sfrench@samba.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.