Message ID | 20170207131841.GC31552@mwanda (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
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>
2017-02-07 5:18 GMT-08:00 Dan Carpenter <dan.carpenter@oracle.com>: > 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 Good catch, thanks! Reviewed-by: Pavel Shilovsky <pshilov@microsoft.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;
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> -- 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