| Message ID | 20200213021447.24819-1-lsahlber@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cifs: make sure we do not overflow the max EA buffer size | expand |
We should be allowing these to be larger than ~16000 bytes Should be XATTR_SIZE_MAX 65536 but that can be done with different patch On Wed, Feb 12, 2020 at 8:15 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > RHBZ: 1752437 > > Before we add a new EA we should check that this will not overflow > the maximum buffer we have available to read the EAs back. > Otherwise we can get into a situation where the EAs are so big that > we can not read them back to the client and thus we can not list EAs > anymore or delete them. > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > --- > fs/cifs/smb2ops.c | 35 ++++++++++++++++++++++++++++++++++- > 1 file changed, 34 insertions(+), 1 deletion(-) > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index baa825f4cec0..3c76f69f4ca7 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -1116,7 +1116,8 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, > void *data[1]; > struct smb2_file_full_ea_info *ea = NULL; > struct kvec close_iov[1]; > - int rc; > + struct smb2_query_info_rsp *rsp; > + int rc, used_len = 0; > > if (smb3_encryption_required(tcon)) > flags |= CIFS_TRANSFORM_REQ; > @@ -1139,6 +1140,38 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, > cifs_sb); > if (rc == -ENODATA) > goto sea_exit; > + } else { > + /* If we are adding a attribute we should first check > + * if there will be enough space available to store > + * the new EA. If not we should not add it since we > + * would not be able to even read the EAs back. > + */ > + rc = smb2_query_info_compound(xid, tcon, utf16_path, > + FILE_READ_EA, > + FILE_FULL_EA_INFORMATION, > + SMB2_O_INFO_FILE, > + CIFSMaxBufSize - > + MAX_SMB2_CREATE_RESPONSE_SIZE - > + MAX_SMB2_CLOSE_RESPONSE_SIZE, > + &rsp_iov[1], &resp_buftype[1], cifs_sb); > + if (rc == 0) { > + rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base; > + used_len = rsp->OutputBufferLength; > + } > + free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base); > + resp_buftype[1] = CIFS_NO_BUFFER; > + memset(&rsp_iov[1], 0, sizeof(rsp_iov[1])); > + rc = 0; > + > + /* Use a fudge factor of 256 bytes in case we collide > + * with a different set_EAs command. > + */ > + if(CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE - > + MAX_SMB2_CLOSE_RESPONSE_SIZE - 256 < > + used_len + ea_name_len + ea_value_len + 1) { > + rc = -ENOSPC; > + goto sea_exit; > + } > } > } > > -- > 2.13.6 >
We can't receive packets bigger that 16k in the memory pool buffers. In order to support bigger response buffer we would need to allocate individual pages and receive the packet directly into them (like we do for writes). -- Best regards, Pavel Shilovsky чт, 13 февр. 2020 г. в 22:14, Steve French <smfrench@gmail.com>: > > We should be allowing these to be larger than ~16000 bytes > > Should be XATTR_SIZE_MAX 65536 > > but that can be done with different patch > > On Wed, Feb 12, 2020 at 8:15 PM Ronnie Sahlberg <lsahlber@redhat.com> wrote: > > > > RHBZ: 1752437 > > > > Before we add a new EA we should check that this will not overflow > > the maximum buffer we have available to read the EAs back. > > Otherwise we can get into a situation where the EAs are so big that > > we can not read them back to the client and thus we can not list EAs > > anymore or delete them. > > > > Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > > --- > > fs/cifs/smb2ops.c | 35 ++++++++++++++++++++++++++++++++++- > > 1 file changed, 34 insertions(+), 1 deletion(-) > > > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > > index baa825f4cec0..3c76f69f4ca7 100644 > > --- a/fs/cifs/smb2ops.c > > +++ b/fs/cifs/smb2ops.c > > @@ -1116,7 +1116,8 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, > > void *data[1]; > > struct smb2_file_full_ea_info *ea = NULL; > > struct kvec close_iov[1]; > > - int rc; > > + struct smb2_query_info_rsp *rsp; > > + int rc, used_len = 0; > > > > if (smb3_encryption_required(tcon)) > > flags |= CIFS_TRANSFORM_REQ; > > @@ -1139,6 +1140,38 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, > > cifs_sb); > > if (rc == -ENODATA) > > goto sea_exit; > > + } else { > > + /* If we are adding a attribute we should first check > > + * if there will be enough space available to store > > + * the new EA. If not we should not add it since we > > + * would not be able to even read the EAs back. > > + */ > > + rc = smb2_query_info_compound(xid, tcon, utf16_path, > > + FILE_READ_EA, > > + FILE_FULL_EA_INFORMATION, > > + SMB2_O_INFO_FILE, > > + CIFSMaxBufSize - > > + MAX_SMB2_CREATE_RESPONSE_SIZE - > > + MAX_SMB2_CLOSE_RESPONSE_SIZE, > > + &rsp_iov[1], &resp_buftype[1], cifs_sb); > > + if (rc == 0) { > > + rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base; > > + used_len = rsp->OutputBufferLength; > > + } > > + free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base); > > + resp_buftype[1] = CIFS_NO_BUFFER; > > + memset(&rsp_iov[1], 0, sizeof(rsp_iov[1])); > > + rc = 0; > > + > > + /* Use a fudge factor of 256 bytes in case we collide > > + * with a different set_EAs command. > > + */ > > + if(CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE - > > + MAX_SMB2_CLOSE_RESPONSE_SIZE - 256 < > > + used_len + ea_name_len + ea_value_len + 1) { > > + rc = -ENOSPC; > > + goto sea_exit; > > + } > > } > > } > > > > -- > > 2.13.6 > > > > > -- > Thanks, > > Steve
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index baa825f4cec0..3c76f69f4ca7 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -1116,7 +1116,8 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, void *data[1]; struct smb2_file_full_ea_info *ea = NULL; struct kvec close_iov[1]; - int rc; + struct smb2_query_info_rsp *rsp; + int rc, used_len = 0; if (smb3_encryption_required(tcon)) flags |= CIFS_TRANSFORM_REQ; @@ -1139,6 +1140,38 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, cifs_sb); if (rc == -ENODATA) goto sea_exit; + } else { + /* If we are adding a attribute we should first check + * if there will be enough space available to store + * the new EA. If not we should not add it since we + * would not be able to even read the EAs back. + */ + rc = smb2_query_info_compound(xid, tcon, utf16_path, + FILE_READ_EA, + FILE_FULL_EA_INFORMATION, + SMB2_O_INFO_FILE, + CIFSMaxBufSize - + MAX_SMB2_CREATE_RESPONSE_SIZE - + MAX_SMB2_CLOSE_RESPONSE_SIZE, + &rsp_iov[1], &resp_buftype[1], cifs_sb); + if (rc == 0) { + rsp = (struct smb2_query_info_rsp *)rsp_iov[1].iov_base; + used_len = rsp->OutputBufferLength; + } + free_rsp_buf(resp_buftype[1], rsp_iov[1].iov_base); + resp_buftype[1] = CIFS_NO_BUFFER; + memset(&rsp_iov[1], 0, sizeof(rsp_iov[1])); + rc = 0; + + /* Use a fudge factor of 256 bytes in case we collide + * with a different set_EAs command. + */ + if(CIFSMaxBufSize - MAX_SMB2_CREATE_RESPONSE_SIZE - + MAX_SMB2_CLOSE_RESPONSE_SIZE - 256 < + used_len + ea_name_len + ea_value_len + 1) { + rc = -ENOSPC; + goto sea_exit; + } } }
RHBZ: 1752437 Before we add a new EA we should check that this will not overflow the maximum buffer we have available to read the EAs back. Otherwise we can get into a situation where the EAs are so big that we can not read them back to the client and thus we can not list EAs anymore or delete them. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> --- fs/cifs/smb2ops.c | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-)