----- Original Message -----
> From: "Steve French" <smfrench@xxxxxxxxx>
> To: "Ronnie Sahlberg" <lsahlber@xxxxxxxxxx>
> Cc: "linux-cifs" <linux-cifs@xxxxxxxxxxxxxxx>
> Sent: Thursday, 19 April, 2018 11:50:35 AM
> Subject: Re: [PATCH] cifs: do not allow creating sockets in SMB1
>
> changing it to EPERM worked - but I think the check for the valid
> types needs to be moved earlier so we don't leave the newly created
> file around on failure. My revised patch (changing EOPNOTSUPP to
> EPERM) does work, but ... better if it failed before creating the
> file.
Yes, we want it to fail before it creates the file.
>
> # python -c "import socket as s; sock = s.socket(s.AF_UNIX);
> sock.bind('/mnt1/somesoc3ket')"
> Traceback (most recent call last):
> File "<string>", line 1, in <module>
> File "/usr/lib/python2.7/socket.py", line 228, in meth
> return getattr(self._sock,name)(*args)
> socket.error: [Errno 1] Operation not permitted
>
> On Wed, Apr 18, 2018 at 8:42 PM, Steve French <smfrench@xxxxxxxxx> wrote:
> > I think the safer patch is to catch all invalid special file types
> > (not just S_ISSOCK) but putting in the missing "else" clause (see
> > below) - but when I tested it I got a different return code than
> > expected mapped by the caller ... so perhaps we need to return a
> > different return code other than EOPNOTSUPP
> >
> > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> > index 81ba6e0d88d8..ebb40ad7ec7f 100644
> > --- a/fs/cifs/dir.c
> > +++ b/fs/cifs/dir.c
> > @@ -742,7 +742,8 @@ int cifs_mknod(struct inode *inode, struct dentry
> > *direntry, umode_t mode,
> > pdev->minor = cpu_to_le64(MINOR(device_number));
> > rc = tcon->ses->server->ops->sync_write(xid, &fid,
> > &io_parms,
> > &bytes_written,
> > iov, 1);
> > - } /* else if (S_ISFIFO) */
> > + } else
> > + rc = -EOPNOTSUPP;
> > tcon->ses->server->ops->close(xid, tcon, &fid);
> > d_drop(direntry);
> >
> >
> > On Wed, Apr 18, 2018 at 7:52 PM, Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> > wrote:
> >> RHBZ: 1453123
> >>
> >> Since at least the 3.10 kernel and likely a lot earlier we have
> >> not been able to create unix domain sockets in a cifs share.
> >> Trying to create a socket, for example using the af_unix command from
> >> xfstests will cause :
> >> BUG: unable to handle kernel NULL pointer dereference at 00000000
> >> 00000040
> >>
> >> Since no one uses or depends on being able to create unix domains sockets
> >> on a cifs share the easiest fix to stop this vulnerability is to simply
> >> disallow creation of such sockets.
> >>
> >> Reported-by: Eryu Guan <eguan@xxxxxxxxxx>
> >> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> >> ---
> >> fs/cifs/dir.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> >> index 81ba6e0d88d8..632a35be85ea 100644
> >> --- a/fs/cifs/dir.c
> >> +++ b/fs/cifs/dir.c
> >> @@ -684,6 +684,9 @@ int cifs_mknod(struct inode *inode, struct dentry
> >> *direntry, umode_t mode,
> >> goto mknod_out;
> >> }
> >>
> >> + if (S_ISSOCK(mode))
> >> + return -EINVAL;
> >> +
> >> if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
> >> goto mknod_out;
> >>
> >> --
> >> 2.13.3
> >>
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve
>
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
