From patchwork Fri Jun 30 07:21:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Po-Hsu Lin X-Patchwork-Id: 782690 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3wzSdM0Vtwz9s8P; Fri, 30 Jun 2017 17:21:43 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical-com.20150623.gappssmtp.com header.i=@canonical-com.20150623.gappssmtp.com header.b="mWB7qhWr"; dkim-atps=neutral Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.76) (envelope-from ) id 1dQqFG-0006BB-Lj; Fri, 30 Jun 2017 07:21:38 +0000 Received: from mail-pg0-f45.google.com ([74.125.83.45]) by huckleberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1dQqF8-00063P-T8 for kernel-team@lists.ubuntu.com; Fri, 30 Jun 2017 07:21:31 +0000 Received: by mail-pg0-f45.google.com with SMTP id f127so59615297pgc.0 for ; Fri, 30 Jun 2017 00:21:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical-com.20150623.gappssmtp.com; s=20150623; h=from:to:subject:date:message-id:in-reply-to:references; bh=gbdIodpvSog9tsvNlzze2OjMyJ+nUkMByNapEJhhi6Y=; b=mWB7qhWrKxfYvxBB1Q+bzSXRWKu2i0bgW+QsvmgTxKv+uRAYz+A1qmagA80BeFxMC7 gbfsQwyF2/CppHXQ/nhp+V11c+JOmuIDHUH0sUaN079l0C6PCfKNVwJX1QgL41i4IVgT ENaDhSrFff8jGxhBtY6PQLT1LWLwWFLV8AYRm5yePCiLkjfFTgjP2HEYVLBlnnp62HwR x5smMYM7Od2dDwoCFB4aU6e01Ig2dg/3GYJTwN7Y6PecdK+9HK2LtgU5ADrdqj/bDoCU nzYki50JF2vmJaSC2XJ4rxmdZ4dHyUz+zkyojJwaFvOx3L+wc28nJNm1GvFywUPvtSYj iuwg== 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=gbdIodpvSog9tsvNlzze2OjMyJ+nUkMByNapEJhhi6Y=; b=k4ndGrQjXye59VVN6gsxvYTaPLhmnPLaPWKrhSEKCK4fUHRR6EczV0x+Mc5YktvjO4 SV8AFyvgWA8YI/ay2EROJntu9O6wCCviWXw5RAyGrRvDoIpuql3nfGTPAHxHmnaIiR0h Pb4u0/zNVonF8HsRdejWZ0d6IFqodnoVW7PD/3B0Zd6Lt4oohTMiUmYzRKk9N1Qi5D8Y JAn62fyoNpLp1axlMyI+MdEZihm6KMidWg5kFEGCxDefriJU8L4sf9xjR3fXaANFQqv+ gbI7mCfJ89+5XdV1LSFNMV388Z0fr+Gt1SBN0PS4TMal8esKT+yfhIk+ysKESk9n+Irm J2kg== X-Gm-Message-State: AKS2vOw15wkieVO8HMc7ewYw3nUxXADA7V8/RDJkat8htyb691+VkUSt RjzSalmUPsZYUljTpfk= X-Received: by 10.84.196.36 with SMTP id k33mr22066769pld.68.1498807289291; Fri, 30 Jun 2017 00:21:29 -0700 (PDT) Received: from localhost.localdomain ([175.41.48.77]) by smtp.gmail.com with ESMTPSA id x85sm16064989pff.92.2017.06.30.00.21.28 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 30 Jun 2017 00:21:28 -0700 (PDT) From: Po-Hsu Lin To: kernel-team@lists.ubuntu.com Subject: [CVE-2017-2618][PATCH T/Y] selinux: fix off-by-one in setprocattr Date: Fri, 30 Jun 2017 15:21:17 +0800 Message-Id: <1498807277-6635-2-git-send-email-po-hsu.lin@canonical.com> X-Mailer: git-send-email 1.7.9.5 In-Reply-To: <1498807277-6635-1-git-send-email-po-hsu.lin@canonical.com> References: <1498807277-6635-1-git-send-email-po-hsu.lin@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.14 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-bounces@lists.ubuntu.com From: Stephen Smalley CVE-2017-2618 SELinux tries to support setting/clearing of /proc/pid/attr attributes from the shell by ignoring terminating newlines and treating an attribute value that begins with a NUL or newline as an attempt to clear the attribute. However, the test for clearing attributes has always been wrong; it has an off-by-one error, and this could further lead to reading past the end of the allocated buffer since commit bb646cdb12e75d82258c2f2e7746d5952d3e321a ("proc_pid_attr_write(): switch to memdup_user()"). Fix the off-by-one error. Even with this fix, setting and clearing /proc/pid/attr attributes from the shell is not straightforward since the interface does not support multiple write() calls (so shells that write the value and newline separately will set and then immediately clear the attribute, requiring use of echo -n to set the attribute), whereas trying to use echo -n "" to clear the attribute causes the shell to skip the write() call altogether since POSIX says that a zero-length write causes no side effects. Thus, one must use echo -n to set and echo without -n to clear, as in the following example: $ echo -n unconfined_u:object_r:user_home_t:s0 > /proc/$$/attr/fscreate $ cat /proc/$$/attr/fscreate unconfined_u:object_r:user_home_t:s0 $ echo "" > /proc/$$/attr/fscreate $ cat /proc/$$/attr/fscreate Note the use of /proc/$$ rather than /proc/self, as otherwise the cat command will read its own attribute value, not that of the shell. There are no users of this facility to my knowledge; possibly we should just get rid of it. UPDATE: Upon further investigation it appears that a local process with the process:setfscreate permission can cause a kernel panic as a result of this bug. This patch fixes CVE-2017-2618. Signed-off-by: Stephen Smalley [PM: added the update about CVE-2017-2618 to the commit description] Cc: stable@vger.kernel.org # 3.5: d6ea83ec6864e Signed-off-by: Paul Moore Signed-off-by: James Morris (cherry picked from commit 0c461cb727d146c9ef2d3e86214f498b78b7d125) Signed-off-by: Po-Hsu Lin Acked-by: Stefan Bader Acked-by: Thadeu Lima de Souza Cascardo --- security/selinux/hooks.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c index 357762a..d2246a8 100644 --- a/security/selinux/hooks.c +++ b/security/selinux/hooks.c @@ -5548,7 +5548,7 @@ static int selinux_setprocattr(struct task_struct *p, return error; /* Obtain a SID for the context, if one was specified. */ - if (size && str[1] && str[1] != '\n') { + if (size && str[0] && str[0] != '\n') { if (str[size-1] == '\n') { str[size-1] = 0; size--;