3.16.61-rc1 review patch. If anyone has any objections, please let me know.
------------------
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") 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 [bwh: Backported to 3.16: open-code uaccess_kernel()] Signed-off-by: Ben Hutchings ben@decadent.org.uk --- drivers/scsi/sg.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-)
--- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -52,6 +52,7 @@ static int sg_version_num = 30534; /* 2 #include <linux/blktrace_api.h> #include <linux/mutex.h> #include <linux/ratelimit.h> +#include <linux/cred.h> /* for sg_check_file_access() */
#include "scsi.h" #include <scsi/scsi_dbg.h> @@ -215,6 +216,33 @@ static void sg_put_dev(Sg_device *sdp); #define SZ_SG_IOVEC sizeof(sg_iovec_t) #define SZ_SG_REQ_INFO sizeof(sg_req_info_t)
+/* + * 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; @@ -382,6 +410,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, printk("sg_read: %s, count=%d\n", @@ -567,9 +603,11 @@ sg_write(struct file *filp, const char _ struct sg_header old_hdr; sg_io_hdr_t *hp; unsigned char cmnd[MAX_COMMAND_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;