| Message ID | CAH2r5mt-BFGGY1ggFYZ8CXpfRFjigt+pmHZJ2yFO+V8XZj=epw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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.
# 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@gmail.com> 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@redhat.com> 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@redhat.com>
>> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
>> ---
>> 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
----- Original Message ----- > From: "Steve French" <smfrench@gmail.com> > To: "Ronnie Sahlberg" <lsahlber@redhat.com> > Cc: "linux-cifs" <linux-cifs@vger.kernel.org> > 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@gmail.com> 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@redhat.com> > > 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@redhat.com> > >> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> > >> --- > >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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);