* [PATCH] cifs: make sure we do not overflow the max EA buffer size
@ 2020-02-13 2:14 Ronnie Sahlberg
2020-02-14 6:14 ` Steve French
0 siblings, 1 reply; 3+ messages in thread
From: Ronnie Sahlberg @ 2020-02-13 2:14 UTC (permalink / raw)
To: linux-cifs; +Cc: Ronnie Sahlberg
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cifs: make sure we do not overflow the max EA buffer size
2020-02-13 2:14 [PATCH] cifs: make sure we do not overflow the max EA buffer size Ronnie Sahlberg
@ 2020-02-14 6:14 ` Steve French
2020-02-14 19:04 ` Pavel Shilovsky
0 siblings, 1 reply; 3+ messages in thread
From: Steve French @ 2020-02-14 6:14 UTC (permalink / raw)
To: Ronnie Sahlberg; +Cc: linux-cifs
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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cifs: make sure we do not overflow the max EA buffer size
2020-02-14 6:14 ` Steve French
@ 2020-02-14 19:04 ` Pavel Shilovsky
0 siblings, 0 replies; 3+ messages in thread
From: Pavel Shilovsky @ 2020-02-14 19:04 UTC (permalink / raw)
To: Steve French; +Cc: Ronnie Sahlberg, linux-cifs
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-02-14 19:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-13 2:14 [PATCH] cifs: make sure we do not overflow the max EA buffer size Ronnie Sahlberg
2020-02-14 6:14 ` Steve French
2020-02-14 19:04 ` Pavel Shilovsky
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).