From patchwork Fri Oct 12 16:09:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kleber Sacilotto de Souza X-Patchwork-Id: 983163 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42Wt8T3Rvhz9s3Z; Sat, 13 Oct 2018 03:10:01 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1gB00f-0002Yv-Ci; Fri, 12 Oct 2018 16:09:53 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1gB00d-0002YW-5r for kernel-team@lists.ubuntu.com; Fri, 12 Oct 2018 16:09:51 +0000 Received: from mail-wm1-f69.google.com ([209.85.128.69]) by youngberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1gB00c-0007zp-Uq for kernel-team@lists.ubuntu.com; Fri, 12 Oct 2018 16:09:50 +0000 Received: by mail-wm1-f69.google.com with SMTP id j124-v6so7191302wmd.4 for ; Fri, 12 Oct 2018 09:09:50 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=63kFgqp3i6f8apk/EhtokfDb0fCqGkvNVJyUxkOELrA=; b=drj1CnJ0Bpj2Hw8nZB/mHJRh6EiTszKML6DMuT/PRYwByZpL46rCfvwkabSR8QqE5O hplIqCiz6XSKyZ8cwgemWy8oQM7zgnEwyStYTHpe8GhTxZPP1uh4CDAC5zNrMfoIRxdl J9ma4mtr9v5hamRB/ESvp3Gmk/b+0hhJcK+feNWKBapqVDLx0BuvrpRduC6rg+d1nejs d7GmL10W4YspahuuUvKPE45V2hTwpePcUM9W+XJmInm2NG7x4SlsIlRVp/59K+4vlDTt U+8F/OKHzYrw/WgoEjNe1a1t24HedkUGIacNc3KmxnUVfhDrRo5tukwpKLsgbJMjQR/G g9ZQ== X-Gm-Message-State: ABuFfoi2oj2YCz1zrWx0ZBWqnTs7JRJxyxJcyx1lKWTZSs2rwh+E5oAl oUdmWod+fOb0Nuo3n3D+oP3PReCrMlmDoFfSaxP7+ipYOqJoOiC/axRLO1CkIyItsYgzvFugX1W LYJKh7ori42GwoH3kLlY7/xwoKjDcWmf+HxkYEm1OXA== X-Received: by 2002:a1c:be09:: with SMTP id o9-v6mr5749728wmf.109.1539360590270; Fri, 12 Oct 2018 09:09:50 -0700 (PDT) X-Google-Smtp-Source: ACcGV62Qh8YwMzffuffkgLqG5P7lVN/oy/+XrBMWjlMeCAcqaEX/YV2IJutEqkNqJxnHLEWB/2BQKg== X-Received: by 2002:a1c:be09:: with SMTP id o9-v6mr5749713wmf.109.1539360589952; Fri, 12 Oct 2018 09:09:49 -0700 (PDT) Received: from localhost ([212.121.131.210]) by smtp.gmail.com with ESMTPSA id w2-v6sm1220390wre.57.2018.10.12.09.09.48 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 12 Oct 2018 09:09:49 -0700 (PDT) From: Kleber Sacilotto de Souza To: kernel-team@lists.ubuntu.com Subject: [SRU][Trusty][PATCH 1/1] scsi: sg: mitigate read/write abuse Date: Fri, 12 Oct 2018 18:09:45 +0200 Message-Id: <20181012160946.23935-2-kleber.souza@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181012160946.23935-1-kleber.souza@canonical.com> References: <20181012160946.23935-1-kleber.souza@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jann Horn 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: Signed-off-by: Jann Horn Acked-by: Douglas Gilbert Signed-off-by: Martin K. Petersen CVE-2017-13168 (backported from commit 26b5b874aff5659a7e26e5b1997e3df2c41fa7fd) [ kleber: on 3.13, uaccess_kernel() and current_real_cred() helpers hadn't been implemented yet. ] Signed-off-by: Kleber Sacilotto de Souza --- drivers/scsi/sg.c | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c index 7b99026238eb..ebf6b0f6dfe5 100644 --- a/drivers/scsi/sg.c +++ b/drivers/scsi/sg.c @@ -52,6 +52,7 @@ static int sg_version_num = 30534; /* 2 digits for each component */ #include #include #include +#include /* for sg_check_file_access() */ #include "scsi.h" #include @@ -216,6 +217,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; @@ -383,6 +411,14 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos) 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", @@ -568,9 +604,11 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos) 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;