| From 26b5b874aff5659a7e26e5b1997e3df2c41fa7fd Mon Sep 17 00:00:00 2001 |
| From: Jann Horn <jannh@google.com> |
| Date: Mon, 25 Jun 2018 16:25:44 +0200 |
| Subject: scsi: sg: mitigate read/write abuse |
| |
| From: Jann Horn <jannh@google.com> |
| |
| commit 26b5b874aff5659a7e26e5b1997e3df2c41fa7fd upstream. |
| |
| As Al Viro noted in commit 128394eff343 ("sg_write()/bsg_write() is not fit |
| to be called under KERNEL_DS"), sg improperly accesses userspace memory |
| outside the provided buffer, permitting kernel memory corruption via |
| splice(). But it doesn't just do it on ->write(), also on ->read(). |
| |
| As a band-aid, make sure that the ->read() and ->write() handlers can not |
| be called in weird contexts (kernel context or credentials different from |
| file opener), like for ib_safe_file_access(). |
| |
| If someone needs to use these interfaces from different security contexts, |
| a new interface should be written that goes through the ->ioctl() handler. |
| |
| I've mostly copypasted ib_safe_file_access() over as sg_safe_file_access() |
| because I couldn't find a good common header - please tell me if you know a |
| better way. |
| |
| [mkp: s/_safe_/_check_/] |
| |
| Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") |
| Cc: <stable@vger.kernel.org> |
| Signed-off-by: Jann Horn <jannh@google.com> |
| Acked-by: Douglas Gilbert <dgilbert@interlog.com> |
| Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| drivers/scsi/sg.c | 42 ++++++++++++++++++++++++++++++++++++++++-- |
| 1 file changed, 40 insertions(+), 2 deletions(-) |
| |
| --- a/drivers/scsi/sg.c |
| +++ b/drivers/scsi/sg.c |
| @@ -51,6 +51,7 @@ static int sg_version_num = 30536; /* 2 |
| #include <linux/atomic.h> |
| #include <linux/ratelimit.h> |
| #include <linux/uio.h> |
| +#include <linux/cred.h> /* for sg_check_file_access() */ |
| |
| #include "scsi.h" |
| #include <scsi/scsi_dbg.h> |
| @@ -221,6 +222,33 @@ static void sg_device_destroy(struct kre |
| sdev_prefix_printk(prefix, (sdp)->device, \ |
| (sdp)->disk->disk_name, fmt, ##a) |
| |
| +/* |
| + * The SCSI interfaces that use read() and write() as an asynchronous variant of |
| + * ioctl(..., SG_IO, ...) are fundamentally unsafe, since there are lots of ways |
| + * to trigger read() and write() calls from various contexts with elevated |
| + * privileges. This can lead to kernel memory corruption (e.g. if these |
| + * interfaces are called through splice()) and privilege escalation inside |
| + * userspace (e.g. if a process with access to such a device passes a file |
| + * descriptor to a SUID binary as stdin/stdout/stderr). |
| + * |
| + * This function provides protection for the legacy API by restricting the |
| + * calling context. |
| + */ |
| +static int sg_check_file_access(struct file *filp, const char *caller) |
| +{ |
| + if (filp->f_cred != current_real_cred()) { |
| + pr_err_once("%s: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n", |
| + caller, task_tgid_vnr(current), current->comm); |
| + return -EPERM; |
| + } |
| + if (unlikely(segment_eq(get_fs(), KERNEL_DS))) { |
| + pr_err_once("%s: process %d (%s) called from kernel context, this is not allowed.\n", |
| + caller, task_tgid_vnr(current), current->comm); |
| + return -EACCES; |
| + } |
| + return 0; |
| +} |
| + |
| static int sg_allow_access(struct file *filp, unsigned char *cmd) |
| { |
| struct sg_fd *sfp = filp->private_data; |
| @@ -405,6 +433,14 @@ sg_read(struct file *filp, char __user * |
| struct sg_header *old_hdr = NULL; |
| int retval = 0; |
| |
| + /* |
| + * This could cause a response to be stranded. Close the associated |
| + * file descriptor to free up any resources being held. |
| + */ |
| + retval = sg_check_file_access(filp, __func__); |
| + if (retval) |
| + return retval; |
| + |
| if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp))) |
| return -ENXIO; |
| SCSI_LOG_TIMEOUT(3, sg_printk(KERN_INFO, sdp, |
| @@ -592,9 +628,11 @@ sg_write(struct file *filp, const char _ |
| struct sg_header old_hdr; |
| sg_io_hdr_t *hp; |
| unsigned char cmnd[SG_MAX_CDB_SIZE]; |
| + int retval; |
| |
| - if (unlikely(segment_eq(get_fs(), KERNEL_DS))) |
| - return -EINVAL; |
| + retval = sg_check_file_access(filp, __func__); |
| + if (retval) |
| + return retval; |
| |
| if ((!(sfp = (Sg_fd *) filp->private_data)) || (!(sdp = sfp->parentdp))) |
| return -ENXIO; |