| Message ID | 20171020124938.9913-2-ddiss@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] SMB: fix leak of validate negotiate info response buffer | expand |
> From: "David Disseldorp" <ddiss@suse.de> > To: "Shu Wang" <shuwang@redhat.com>, sfrench@samba.org, linux-cifs@vger.kernel.org, samba-technical@lists.samba.org > Cc: "David Disseldorp" <ddiss@suse.de> > Sent: Friday, October 20, 2017 8:49:37 PM > Subject: [PATCH 1/2] SMB: fix leak of validate negotiate info response buffer > > Fixes: ff1c038addc4 ("Check SMB3 dialects against downgrade attacks") > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > fs/cifs/smb2pdu.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 6f0e6343c15e..052ab5dee6b6 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -648,7 +648,7 @@ int smb3_validate_negotiate(const unsigned int xid, > struct cifs_tcon *tcon) > { > int rc = 0; > struct validate_negotiate_info_req vneg_inbuf; > - struct validate_negotiate_info_rsp *pneg_rsp; > + struct validate_negotiate_info_rsp *pneg_rsp = NULL; > u32 rsplen; > u32 inbuflen; /* max of 4 dialects */ > SMB2_ioctl will set pneg_rsp pointer to NULL, so it won't really cause any issue. Anyway, looks good to me. 1879 SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, 1880 >--- u64 volatile_fid, u32 opcode, bool is_fsctl, bool use_ipc, 1881 >--- char *in_data, u32 indatalen, 1882 >--- char **out_data, u32 *plen /* returned data len */) 1883 { ........ 1897 >---if (out_data != NULL) 1898 >--->---*out_data = NULL; 1899 -- 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
Hi Shu Wang, On Fri, 20 Oct 2017 22:49:58 -0400 (EDT), Shu Wang wrote: > > --- a/fs/cifs/smb2pdu.c > > +++ b/fs/cifs/smb2pdu.c > > @@ -648,7 +648,7 @@ int smb3_validate_negotiate(const unsigned int xid, > > struct cifs_tcon *tcon) > > { > > int rc = 0; > > struct validate_negotiate_info_req vneg_inbuf; > > - struct validate_negotiate_info_rsp *pneg_rsp; > > + struct validate_negotiate_info_rsp *pneg_rsp = NULL; > > u32 rsplen; > > u32 inbuflen; /* max of 4 dialects */ > > > > SMB2_ioctl will set pneg_rsp pointer to NULL, so it won't really > cause any issue. Anyway, looks good to me. Yeah, this hunk is unnecessary, but thought it might be helpful if someone in future wants to jump to the error path prior to the SMB2_ioctl() call. @Steve: feel free to drop it if you prefer. Cheers, David -- 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
diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 6f0e6343c15e..052ab5dee6b6 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -648,7 +648,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) { int rc = 0; struct validate_negotiate_info_req vneg_inbuf; - struct validate_negotiate_info_rsp *pneg_rsp; + struct validate_negotiate_info_rsp *pneg_rsp = NULL; u32 rsplen; u32 inbuflen; /* max of 4 dialects */ @@ -728,7 +728,7 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) /* relax check since Mac returns max bufsize allowed on ioctl */ if (rsplen > CIFSMaxBufSize) - return -EIO; + goto err_rsp_free; } /* check validate negotiate info response matches what we got earlier */ @@ -747,10 +747,13 @@ int smb3_validate_negotiate(const unsigned int xid, struct cifs_tcon *tcon) /* validate negotiate successful */ cifs_dbg(FYI, "validate negotiate info successful\n"); + kfree(pneg_rsp); return 0; vneg_out: cifs_dbg(VFS, "protocol revalidation - security settings mismatch\n"); +err_rsp_free: + kfree(pneg_rsp); return -EIO; }
Fixes: ff1c038addc4 ("Check SMB3 dialects against downgrade attacks") Signed-off-by: David Disseldorp <ddiss@suse.de> --- fs/cifs/smb2pdu.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)