Linux-CIFS Archive on lore.kernel.org
 help / color / Atom feed
From: Pavel Shilovsky <piastryyy@gmail.com>
To: Steve French <smfrench@gmail.com>
Cc: Ronnie Sahlberg <lsahlber@redhat.com>,
	linux-cifs <linux-cifs@vger.kernel.org>
Subject: Re: [PATCH] cifs: make sure we do not overflow the max EA buffer size
Date: Fri, 14 Feb 2020 11:04:47 -0800
Message-ID: <CAKywueQ_=r7m_XDxjyH1DON3Smz-q3LSJDuYKG-AG8npH7hyDg@mail.gmail.com> (raw)
In-Reply-To: <CAH2r5ms0Bz6gVS1guJS6_=3fwQSbdd2yOh7PKJCkrvqFeyUgnQ@mail.gmail.com>

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

      reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-13  2:14 Ronnie Sahlberg
2020-02-14  6:14 ` Steve French
2020-02-14 19:04   ` Pavel Shilovsky [this message]

Reply instructions:

You may reply publically 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='CAKywueQ_=r7m_XDxjyH1DON3Smz-q3LSJDuYKG-AG8npH7hyDg@mail.gmail.com' \
    --to=piastryyy@gmail.com \
    --cc=linux-cifs@vger.kernel.org \
    --cc=lsahlber@redhat.com \
    --cc=smfrench@gmail.com \
    /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

Linux-CIFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-cifs/0 linux-cifs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-cifs linux-cifs/ https://lore.kernel.org/linux-cifs \
		linux-cifs@vger.kernel.org
	public-inbox-index linux-cifs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-cifs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git