cifs: do not allow creating sockets in SMB1

Message ID 20180419024808.15957-2-lsahlber@redhat.com
State New
Headers show
Series
  • cifs: fix kernel NULL deref when creating sockets
Related show

Commit Message

Ronnie Sahlberg April 19, 2018, 2:48 a.m.
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
not allow creation of any other special files than char or block devices
when sfu is used.

Reported-by: Eryu Guan <eguan@redhat.com>
Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/dir.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Steve French April 19, 2018, 3:12 a.m. | #1
Looks fine to me - only question is what return code is supposed to be
returned - any luck tracking down what other fs return on unsupported
types?

On Wed, Apr 18, 2018 at 9:48 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
> not allow creation of any other special files than char or block devices
> when sfu is used.
>
> Reported-by: Eryu Guan <eguan@redhat.com>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/dir.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 81ba6e0d88d8..f0a759dd6817 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_ISCHR(mode) && !S_ISBLK(mode))
> +               return -EPERM;
> +
>         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
>                 goto mknod_out;
>
> @@ -742,7 +745,7 @@ 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) */
> +       }
>         tcon->ses->server->ops->close(xid, tcon, &fid);
>         d_drop(direntry);
>
> --
> 2.13.3
>
Steve French April 19, 2018, 3:19 a.m. | #2
On Wed, Apr 18, 2018 at 10:18 PM, ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
> re-send in ascii
>
> man 2 mknod says EPERM is what we should use here.:
>
>        EPERM  mode requested creation of something other than a regular
> file,
>               FIFO  (named pipe), or UNIX domain socket, and the caller is
> not
>               privileged (Linux: does not have the CAP_MKNOD capability);
> also
>               returned  if the filesystem containing pathname does not
> support
>               the type of node requested.

ok - looks good . thx

> On Thu, Apr 19, 2018 at 1:12 PM, Steve French <smfrench@gmail.com> wrote:
>>
>> Looks fine to me - only question is what return code is supposed to be
>> returned - any luck tracking down what other fs return on unsupported
>> types?
>>
>> On Wed, Apr 18, 2018 at 9:48 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

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
Steve French April 19, 2018, 3:43 a.m. | #3
merged into cifs-2.6.git for-next (but I did update the commit
description - you might doublecheck that you agree with it).

On Wed, Apr 18, 2018 at 9:48 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
> not allow creation of any other special files than char or block devices
> when sfu is used.
>
> Reported-by: Eryu Guan <eguan@redhat.com>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/dir.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> index 81ba6e0d88d8..f0a759dd6817 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_ISCHR(mode) && !S_ISBLK(mode))
> +               return -EPERM;
> +
>         if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
>                 goto mknod_out;
>
> @@ -742,7 +745,7 @@ 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) */
> +       }
>         tcon->ses->server->ops->close(xid, tcon, &fid);
>         d_drop(direntry);
>
> --
> 2.13.3
>

Patch

diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 81ba6e0d88d8..f0a759dd6817 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_ISCHR(mode) && !S_ISBLK(mode))
+		return -EPERM;
+
 	if (!(cifs_sb->mnt_cifs_flags & CIFS_MOUNT_UNX_EMUL))
 		goto mknod_out;
 
@@ -742,7 +745,7 @@  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) */
+	}
 	tcon->ses->server->ops->close(xid, tcon, &fid);
 	d_drop(direntry);