le32_to_cpu() returns unsigned - see below from
uapi/linux/byteorder/little_endian.h:
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
On Tue, Apr 16, 2019 at 3:30 PM Tom Talpey <ttalpey@xxxxxxxxxxxxx> wrote:
>
> > -----Original Message-----
> > From: linux-cifs-owner@xxxxxxxxxxxxxxx <linux-cifs-owner@xxxxxxxxxxxxxxx> On
> > Behalf Of Ronnie Sahlberg
> > Sent: Sunday, April 14, 2019 10:14 PM
> > To: linux-cifs <linux-cifs@xxxxxxxxxxxxxxx>
> > Cc: Steve French <smfrench@xxxxxxxxx>; Ronnie Sahlberg
> > <lsahlber@xxxxxxxxxx>
> > Subject: [PATCH] cifs: fix bi-directional fsctl passthrough calls
> >
> > SMB2 Ioctl responses from servers may respond with both the request blob
> > from
> > the client followed by the actual reply blob for ioctls that are bi-directional.
> >
> > In that case we can not assume that the reply blob comes immediately after
> > the
> > ioctl response structure.
> >
> > This fixes FSCTLs such as SMB2:FSCTL_QUERY_ALLOCATED_RANGES
> >
> > Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > ---
> > fs/cifs/smb2ops.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> > index 0b7e2be2a781..2824b97c5869 100644
> > --- a/fs/cifs/smb2ops.c
> > +++ b/fs/cifs/smb2ops.c
> > @@ -1462,12 +1462,19 @@ smb2_ioctl_query_info(const unsigned int xid,
> > io_rsp = (struct smb2_ioctl_rsp *)rsp_iov[1].iov_base;
> > if (le32_to_cpu(io_rsp->OutputCount) <
> > qi.input_buffer_length)
> > qi.input_buffer_length = le32_to_cpu(io_rsp-
> > >OutputCount);
> > + if (qi.input_buffer_length > 0 &&
> > + le32_to_cpu(io_rsp->OutputOffset) + qi.input_buffer_length
> > > rsp_iov[1].iov_len) {
>
> I hope this isn't a dumb question, but I can't readily figure it out due to
> multiple layers of #include nesting. Is le32_to_cpu() signed or unsigned?
> If it's signed, this validation could still be attacked.
>
> And second question, is the fsctl output payload always guaranteed to land
> in rsp_iov[1]?
>
> Tom.
>
> > + rc = -EFAULT;
> > + goto iqinf_exit;
> > + }
> > if (copy_to_user(&pqi->input_buffer_length,
> > &qi.input_buffer_length,
> > sizeof(qi.input_buffer_length))) {
> > rc = -EFAULT;
> > goto iqinf_exit;
> > }
> > - if (copy_to_user(pqi + 1, &io_rsp[1], qi.input_buffer_length)) {
> > + if (copy_to_user((char *)pqi + sizeof(struct smb_query_info),
> > + (char *)io_rsp + le32_to_cpu(io_rsp-
> > >OutputOffset),
> > + qi.input_buffer_length)) {
> > rc = -EFAULT;
> > goto iqinf_exit;
> > }
> > --
> > 2.13.6
>
--
Thanks,
Steve
