From patchwork Wed Apr 24 16:37:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 10915207 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id E8F3913B5 for ; Wed, 24 Apr 2019 16:38:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D90EF28AB6 for ; Wed, 24 Apr 2019 16:38:07 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id CA98228AE6; Wed, 24 Apr 2019 16:38:07 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6B6BF28AB6 for ; Wed, 24 Apr 2019 16:38:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732585AbfDXQiG (ORCPT ); Wed, 24 Apr 2019 12:38:06 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:43055 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731700AbfDXQiG (ORCPT ); Wed, 24 Apr 2019 12:38:06 -0400 Received: by mail-pl1-f193.google.com with SMTP id n8so9543984plp.10 for ; Wed, 24 Apr 2019 09:38:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=yK7KGyF6dcgyF8+VLqnKkfFFI5sTu/wy7hlOVceGxoo=; b=RBOirwOb3Ya7gk2LD0kWtKEUQ22WzVZqXR1xEvR2eCGtH4k8JLwqKMdgxsJYbG6vg7 WTrIghWf3KFQY4HJmf/07OjLu3nz07eZ0B5v/yXzl82fFcvm49lYscFgR+bLBB5wiF5f mQlhjmbSchiELaHYuSL+p1Imw3d2+cwJGUyoI= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=yK7KGyF6dcgyF8+VLqnKkfFFI5sTu/wy7hlOVceGxoo=; b=S4dl1ooyz3bVN7FVftXNI4sJ/TH0/7MhOV4QVAji63SxT4H8QNAnv0/pg0LNWktIHX sJCg4+Tdj6tAY7rUprpMStFha9bcQhV4v/HGMLCZT0ecw78TkSSlcA5MVdXixwz/TpzK XpDEEBbgmcqMboBqjaEkcYqUp0jDtW/Q5wFHB5V39Ku88UN2IuJr26tWKggkR9BIyEEw 7fbjCFFSw5wClbHTr2tIMZ2lozlOJpFigxOjDBWJZ6riqbl+1iRoMp/fIkR8ePVozUvR xRrA0C8afRJcTU0uPS5DEy38UdykedkM4XF8DBYrpyThZK+9+E7/bTr/B8zMxen2NB1J 9+RQ== X-Gm-Message-State: APjAAAXCvvRhmZLt1ZesCca+jJpv9vWbglIlBUfN+n11R1hA4+5mLPZj MLSEmR0mx/RflD8nzLkk0uSTow== X-Google-Smtp-Source: APXvYqxxzUoDVfNqGEb9wutnoTpRSACF5noWmblLcg/Pb+2utXlEbmvHezmxGwUHOasMGQtVCUCRxw== X-Received: by 2002:a17:902:8bc3:: with SMTP id r3mr33857257plo.53.1556123886024; Wed, 24 Apr 2019 09:38:06 -0700 (PDT) Received: from www.outflux.net (173-164-112-133-Oregon.hfc.comcastbusiness.net. [173.164.112.133]) by smtp.gmail.com with ESMTPSA id j6sm24688302pfe.107.2019.04.24.09.38.04 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Apr 2019 09:38:04 -0700 (PDT) From: Kees Cook To: Tycho Andersen Cc: Kees Cook , stable@vger.kernel.org, James Morris , Andy Lutomirski , Will Drewry , linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/2] selftests/seccomp: Prepare for exclusive seccomp flags Date: Wed, 24 Apr 2019 09:37:55 -0700 Message-Id: <20190424163756.40001-2-keescook@chromium.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190424163756.40001-1-keescook@chromium.org> References: <20190424163756.40001-1-keescook@chromium.org> Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Some seccomp flags will become exclusive, so the selftest needs to be adjusted to mask those out and test them individually for the "all flags" tests. Cc: stable@vger.kernel.org # v5.0+ Signed-off-by: Kees Cook Reviewed-by: Tycho Andersen Acked-by: James Morris --- tools/testing/selftests/seccomp/seccomp_bpf.c | 34 ++++++++++++++----- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index f69d2ee29742..5019cdae5d0b 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -2166,11 +2166,14 @@ TEST(detect_seccomp_filter_flags) SECCOMP_FILTER_FLAG_LOG, SECCOMP_FILTER_FLAG_SPEC_ALLOW, SECCOMP_FILTER_FLAG_NEW_LISTENER }; - unsigned int flag, all_flags; + unsigned int exclusive[] = { + SECCOMP_FILTER_FLAG_TSYNC, + SECCOMP_FILTER_FLAG_NEW_LISTENER }; + unsigned int flag, all_flags, exclusive_mask; int i; long ret; - /* Test detection of known-good filter flags */ + /* Test detection of individual known-good filter flags */ for (i = 0, all_flags = 0; i < ARRAY_SIZE(flags); i++) { int bits = 0; @@ -2197,16 +2200,29 @@ TEST(detect_seccomp_filter_flags) all_flags |= flag; } - /* Test detection of all known-good filter flags */ - ret = seccomp(SECCOMP_SET_MODE_FILTER, all_flags, NULL); - EXPECT_EQ(-1, ret); - EXPECT_EQ(EFAULT, errno) { - TH_LOG("Failed to detect that all known-good filter flags (0x%X) are supported!", - all_flags); + /* + * Test detection of all known-good filter flags combined. But + * for the exclusive flags we need to mask them out and try them + * individually for the "all flags" testing. + */ + exclusive_mask = 0; + for (i = 0; i < ARRAY_SIZE(exclusive); i++) + exclusive_mask |= exclusive[i]; + for (i = 0; i < ARRAY_SIZE(exclusive); i++) { + flag = all_flags & ~exclusive_mask; + flag |= exclusive[i]; + + ret = seccomp(SECCOMP_SET_MODE_FILTER, flag, NULL); + EXPECT_EQ(-1, ret); + EXPECT_EQ(EFAULT, errno) { + TH_LOG("Failed to detect that all known-good filter flags (0x%X) are supported!", + flag); + } } - /* Test detection of an unknown filter flag */ + /* Test detection of an unknown filter flags, without exclusives. */ flag = -1; + flag &= ~exclusive_mask; ret = seccomp(SECCOMP_SET_MODE_FILTER, flag, NULL); EXPECT_EQ(-1, ret); EXPECT_EQ(EINVAL, errno) { From patchwork Wed Apr 24 16:37:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 10915211 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id A12E5922 for ; Wed, 24 Apr 2019 16:38:19 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8E61C28AB6 for ; Wed, 24 Apr 2019 16:38:19 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 823D528AE6; Wed, 24 Apr 2019 16:38:19 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0904628AB6 for ; Wed, 24 Apr 2019 16:38:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731345AbfDXQiR (ORCPT ); Wed, 24 Apr 2019 12:38:17 -0400 Received: from mail-pf1-f193.google.com ([209.85.210.193]:46164 "EHLO mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732609AbfDXQiH (ORCPT ); Wed, 24 Apr 2019 12:38:07 -0400 Received: by mail-pf1-f193.google.com with SMTP id 9so9572327pfj.13 for ; Wed, 24 Apr 2019 09:38:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=0PO8cqNNrsc0erE2qFybiWt9ZeKPOIyq2aL7nG116E4=; b=lKNPpcuRi/jRmdBX5Un4UMoF9evOPzRrfzwK/AP3gJqsY1umqamPoaG6/MpT9jgXgL CrgNPHJUsLzhAZiAhiHCvJtf21B0JouJPSD21ExLQPJmu4nu6AQS/j+fsZPdnJ09YX/C nps/moWtLTGWg3c+gBXol4F31ZKIA9SeJ4eNM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=0PO8cqNNrsc0erE2qFybiWt9ZeKPOIyq2aL7nG116E4=; b=fxbiz6IkpduH49FssJSzwXRe0x1PWLPic0cgucgbU4UNSO9kPPJUTGCTE0b1XF6ssf 7Q6CPglkEGeu80nF3f1QAyLEaP4A3qnRi8TrdSXlCKgg82WeHqgulPdEng64n1HDw682 9RrgDNjU9SgjVuwE+nSKFMSkNUpkfnDzhiWoJTpdilaExX2fMmtVF7TUP1jEFK9h1pud aAMHfA0zGpBO9RPEjBDLv/3YWEQscm/EUa4dnenVGXJt3U0K4jdj5jSeBwNE0LMP9D7F G3kQTD8U0FNoz4Dk1Q6NQLIoUQOHD7l794MtMEZOqD1Mi+iCCf7IS+WZMNZgZBOaltcM TCUQ== X-Gm-Message-State: APjAAAXXYVVmxKdTyFKAboXypOCVIzEUX8EEEfpNgwkM0cOl0WUI+dik McVw5uBFKltr9QJI1rEo72mi1w== X-Google-Smtp-Source: APXvYqw+WRueRcdgaAkh3mJqglMzdxBa+Azdx/uYhSx50VoUTeBsE10FaDB+/ehC5KQ8hMhj2SXAKQ== X-Received: by 2002:a65:4343:: with SMTP id k3mr160190pgq.384.1556123886584; Wed, 24 Apr 2019 09:38:06 -0700 (PDT) Received: from www.outflux.net (173-164-112-133-Oregon.hfc.comcastbusiness.net. [173.164.112.133]) by smtp.gmail.com with ESMTPSA id b7sm38220062pfj.67.2019.04.24.09.38.04 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Wed, 24 Apr 2019 09:38:04 -0700 (PDT) From: Kees Cook To: Tycho Andersen Cc: Kees Cook , stable@vger.kernel.org, James Morris , Andy Lutomirski , Will Drewry , linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/2] seccomp: Make NEW_LISTENER and TSYNC flags exclusive Date: Wed, 24 Apr 2019 09:37:56 -0700 Message-Id: <20190424163756.40001-3-keescook@chromium.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20190424163756.40001-1-keescook@chromium.org> References: <20190424163756.40001-1-keescook@chromium.org> Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Tycho Andersen As the comment notes, the return codes for TSYNC and NEW_LISTENER conflict, because they both return positive values, one in the case of success and one in the case of error. So, let's disallow both of these flags together. While this is technically a userspace break, all the users I know of are still waiting on me to land this feature in libseccomp, so I think it'll be safe. Also, at present my use case doesn't require TSYNC at all, so this isn't a big deal to disallow. If someone wanted to support this, a path forward would be to add a new flag like TSYNC_AND_LISTENER_YES_I_UNDERSTAND_THAT_TSYNC_WILL_JUST_RETURN_EAGAIN, but the use cases are so different I don't see it really happening. Finally, it's worth noting that this does actually fix a UAF issue: at the end of seccomp_set_mode_filter(), we have: if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) { if (ret < 0) { listener_f->private_data = NULL; fput(listener_f); put_unused_fd(listener); } else { fd_install(listener, listener_f); ret = listener; } } out_free: seccomp_filter_free(prepared); But if ret > 0 because TSYNC raced, we'll install the listener fd and then free the filter out from underneath it, causing a UAF when the task closes it or dies. This patch also switches the condition to be simply if (ret), so that if someone does add the flag mentioned above, they won't have to remember to fix this too. Reported-by: syzbot+b562969adb2e04af3442@syzkaller.appspotmail.com Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") CC: stable@vger.kernel.org # v5.0+ Signed-off-by: Tycho Andersen Signed-off-by: Kees Cook Acked-by: James Morris --- kernel/seccomp.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/kernel/seccomp.c b/kernel/seccomp.c index df27e499956a..3582eeb59893 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -502,7 +502,10 @@ seccomp_prepare_user_filter(const char __user *user_filter) * * Caller must be holding current->sighand->siglock lock. * - * Returns 0 on success, -ve on error. + * Returns 0 on success, -ve on error, or + * - in TSYNC mode: the pid of a thread which was either not in the correct + * seccomp mode or did not have an ancestral seccomp filter + * - in NEW_LISTENER mode: the fd of the new listener */ static long seccomp_attach_filter(unsigned int flags, struct seccomp_filter *filter) @@ -1258,6 +1261,16 @@ static long seccomp_set_mode_filter(unsigned int flags, if (flags & ~SECCOMP_FILTER_FLAG_MASK) return -EINVAL; + /* + * In the successful case, NEW_LISTENER returns the new listener fd. + * But in the failure case, TSYNC returns the thread that died. If you + * combine these two flags, there's no way to tell whether something + * succeeded or failed. So, let's disallow this combination. + */ + if ((flags & SECCOMP_FILTER_FLAG_TSYNC) && + (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER)) + return -EINVAL; + /* Prepare the new filter before holding any locks. */ prepared = seccomp_prepare_user_filter(filter); if (IS_ERR(prepared)) @@ -1304,7 +1317,7 @@ static long seccomp_set_mode_filter(unsigned int flags, mutex_unlock(¤t->signal->cred_guard_mutex); out_put_fd: if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) { - if (ret < 0) { + if (ret) { listener_f->private_data = NULL; fput(listener_f); put_unused_fd(listener);