From patchwork Wed Jun 17 22:03:21 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610803 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 0851D90 for ; Wed, 17 Jun 2020 22:04:27 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E04392192A for ; Wed, 17 Jun 2020 22:04:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="n6Kfj+jl" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727090AbgFQWE0 (ORCPT ); Wed, 17 Jun 2020 18:04:26 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44930 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727092AbgFQWDe (ORCPT ); Wed, 17 Jun 2020 18:03:34 -0400 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 15C3EC0613EE for ; Wed, 17 Jun 2020 15:03:34 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id b16so1780883pfi.13 for ; Wed, 17 Jun 2020 15:03:34 -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 :mime-version:content-transfer-encoding; bh=oyriWhFwZcOqgM4PO4Jyyu/qJTCulFv+DEEh0q062EY=; b=n6Kfj+jl13q/8FCYxUMTymePBUnKn/5yu6r/9OaYw3FB/UYTfk43YA/BwX7QdxaYfr vEsBjlQ4M0p0EsD5aKsK29tMhTw0QSrUh+FaZzGN136721+wrlrYpdtsYRV15RLkTq9t UtJI8NLB6nd9mBdmPAIkBkvQiZWwibwfwt93A= 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:mime-version:content-transfer-encoding; bh=oyriWhFwZcOqgM4PO4Jyyu/qJTCulFv+DEEh0q062EY=; b=XxhIynSb885AgB81ZTdZMAzl8JYVJdy9ikMZ9Y/mWe2LDn13EsQOnc46pN6FKFV7In Gq9S3GcKfQwTtTNg8f3tm79ZEnvKT9hOVm3Gkcxnq32xd2MWQMNSxJ4SOCa+A1Zywwux dndjs10Lxgf2bW6SydgfXTm9VwSqwSUUOGruXyoTyqURtwQZ64YA/1xiYakqIlhgpfzv Qjeofzi3fdnphX1gp10QbbAaTsv9RdoyQYx8Z6F7Ijuyz6n/RHWJuULC6q80daEJwqwT qH3Exn8E98R2/8HoVt6qN0y5mPrD7jyaLsUTr9pe88N9iHu32mSf/aSDo2jMl04vmEc4 ejXw== X-Gm-Message-State: AOAM532tk3qhjZCX4foLTs9+Fyl6fg62ydSlQ0u1V9Z+mYbCCvPJM6XI 1+SPuk2WdCMseLrQeAfowAB1cg== X-Google-Smtp-Source: ABdhPJxgBh94W9B8rZKjcUmxwEkZ72neBMdKWL2EeGSf3KpDjNoxoQidu3QQqfT0T5Gml8iJeV4xYQ== X-Received: by 2002:aa7:8388:: with SMTP id u8mr753848pfm.253.1592431413571; Wed, 17 Jun 2020 15:03:33 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id c2sm768416pfi.71.2020.06.17.15.03.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:30 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Matt Denton , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 1/7] net/scm: Regularize compat handling of scm_detach_fds() Date: Wed, 17 Jun 2020 15:03:21 -0700 Message-Id: <20200617220327.3731559-2-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Duplicate the cleanups from commit 2618d530dd8b ("net/scm: cleanup scm_detach_fds") into the compat code. Move the check added in commit 1f466e1f15cf ("net: cleanly handle kernel vs user buffers for ->msg_control") to before the compat call, even though it should be impossible for an in-kernel call to also be compat. Correct the int "flags" argument to unsigned int to match fd_install() and similar APIs. Regularize any remaining differences, including a whitespace issue, a checkpatch warning, and add the check from commit 6900317f5eff ("net, scm: fix PaX detected msg_controllen overflow in scm_detach_fds") which fixed an overflow unique to 64-bit. To avoid confusion when comparing the compat handler to the native handler, just include the same check in the compat handler. Fixes: 48a87cc26c13 ("net: netprio: fd passed in SCM_RIGHTS datagram not set correctly") Fixes: d84295067fc7 ("net: net_cls: fd passed in SCM_RIGHTS datagram not set correctly") Signed-off-by: Kees Cook --- include/net/scm.h | 1 + net/compat.c | 55 +++++++++++++++++++++-------------------------- net/core/scm.c | 18 ++++++++-------- 3 files changed, 35 insertions(+), 39 deletions(-) diff --git a/include/net/scm.h b/include/net/scm.h index 1ce365f4c256..581a94d6c613 100644 --- a/include/net/scm.h +++ b/include/net/scm.h @@ -37,6 +37,7 @@ struct scm_cookie { #endif }; +int __scm_install_fd(struct file *file, int __user *ufd, unsigned int o_flags); void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm); void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm); int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm); diff --git a/net/compat.c b/net/compat.c index 5e3041a2c37d..27d477fdcaa0 100644 --- a/net/compat.c +++ b/net/compat.c @@ -281,39 +281,31 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat return 0; } -void scm_detach_fds_compat(struct msghdr *kmsg, struct scm_cookie *scm) +static int scm_max_fds_compat(struct msghdr *msg) { - struct compat_cmsghdr __user *cm = (struct compat_cmsghdr __user *) kmsg->msg_control; - int fdmax = (kmsg->msg_controllen - sizeof(struct compat_cmsghdr)) / sizeof(int); - int fdnum = scm->fp->count; - struct file **fp = scm->fp->fp; - int __user *cmfptr; - int err = 0, i; + if (msg->msg_controllen <= sizeof(struct compat_cmsghdr)) + return 0; + return (msg->msg_controllen - sizeof(struct compat_cmsghdr)) / sizeof(int); +} - if (fdnum < fdmax) - fdmax = fdnum; +void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) +{ + struct compat_cmsghdr __user *cm = + (struct compat_cmsghdr __user *)msg->msg_control; + unsigned int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0; + int fdmax = min_t(int, scm_max_fds_compat(msg), scm->fp->count); + int __user *cmsg_data = CMSG_USER_DATA(cm); + int err = 0, i; - for (i = 0, cmfptr = (int __user *) CMSG_COMPAT_DATA(cm); i < fdmax; i++, cmfptr++) { - int new_fd; - err = security_file_receive(fp[i]); + for (i = 0; i < fdmax; i++) { + err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); if (err) break; - err = get_unused_fd_flags(MSG_CMSG_CLOEXEC & kmsg->msg_flags - ? O_CLOEXEC : 0); - if (err < 0) - break; - new_fd = err; - err = put_user(new_fd, cmfptr); - if (err) { - put_unused_fd(new_fd); - break; - } - /* Bump the usage count and install the file. */ - fd_install(new_fd, get_file(fp[i])); } if (i > 0) { int cmlen = CMSG_COMPAT_LEN(i * sizeof(int)); + err = put_user(SOL_SOCKET, &cm->cmsg_level); if (!err) err = put_user(SCM_RIGHTS, &cm->cmsg_type); @@ -321,16 +313,19 @@ void scm_detach_fds_compat(struct msghdr *kmsg, struct scm_cookie *scm) err = put_user(cmlen, &cm->cmsg_len); if (!err) { cmlen = CMSG_COMPAT_SPACE(i * sizeof(int)); - kmsg->msg_control += cmlen; - kmsg->msg_controllen -= cmlen; + if (msg->msg_controllen < cmlen) + cmlen = msg->msg_controllen; + msg->msg_control += cmlen; + msg->msg_controllen -= cmlen; } } - if (i < fdnum) - kmsg->msg_flags |= MSG_CTRUNC; + + if (i < scm->fp->count || (scm->fp->count && fdmax <= 0)) + msg->msg_flags |= MSG_CTRUNC; /* - * All of the files that fit in the message have had their - * usage counts incremented, so we just free the list. + * All of the files that fit in the message have had their usage counts + * incremented, so we just free the list. */ __scm_destroy(scm); } diff --git a/net/core/scm.c b/net/core/scm.c index 875df1c2989d..6151678c73ed 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -280,7 +280,7 @@ void put_cmsg_scm_timestamping(struct msghdr *msg, struct scm_timestamping_inter } EXPORT_SYMBOL(put_cmsg_scm_timestamping); -static int __scm_install_fd(struct file *file, int __user *ufd, int o_flags) +int __scm_install_fd(struct file *file, int __user *ufd, unsigned int o_flags) { struct socket *sock; int new_fd; @@ -319,29 +319,29 @@ static int scm_max_fds(struct msghdr *msg) void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) { - struct cmsghdr __user *cm - = (__force struct cmsghdr __user*)msg->msg_control; - int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0; + struct cmsghdr __user *cm = + (__force struct cmsghdr __user *)msg->msg_control; + unsigned int o_flags = (msg->msg_flags & MSG_CMSG_CLOEXEC) ? O_CLOEXEC : 0; int fdmax = min_t(int, scm_max_fds(msg), scm->fp->count); int __user *cmsg_data = CMSG_USER_DATA(cm); int err = 0, i; + /* no use for FD passing from kernel space callers */ + if (WARN_ON_ONCE(!msg->msg_control_is_user)) + return; + if (msg->msg_flags & MSG_CMSG_COMPAT) { scm_detach_fds_compat(msg, scm); return; } - /* no use for FD passing from kernel space callers */ - if (WARN_ON_ONCE(!msg->msg_control_is_user)) - return; - for (i = 0; i < fdmax; i++) { err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); if (err) break; } - if (i > 0) { + if (i > 0) { int cmlen = CMSG_LEN(i * sizeof(int)); err = put_user(SOL_SOCKET, &cm->cmsg_level); From patchwork Wed Jun 17 22:03:22 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610805 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id BD1C190 for ; Wed, 17 Jun 2020 22:04:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9937D21924 for ; Wed, 17 Jun 2020 22:04:33 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="khHJTxzO" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727101AbgFQWDe (ORCPT ); Wed, 17 Jun 2020 18:03:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44924 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727089AbgFQWDd (ORCPT ); Wed, 17 Jun 2020 18:03:33 -0400 Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5C571C061794 for ; Wed, 17 Jun 2020 15:03:33 -0700 (PDT) Received: by mail-pf1-x442.google.com with SMTP id 10so1796888pfx.8 for ; Wed, 17 Jun 2020 15:03:33 -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 :mime-version:content-transfer-encoding; bh=eJ8w8RlGTgKwSve8kq67XCo8awUqV+HYghFRfD7mOnw=; b=khHJTxzOcpg3WW5hLAFpnEq6H4kaSxxC0QdAupbyEi/SkYZXIQpq4Sx73NWNUwfQR+ CTqRAET0tfT3MoL8TzGYvwdrDxJ2aCWf+oE63mVvjr8urvd89xQ/N378XJsE04oMtIMi Z2ZYnkIEBKX39kTeCgm+TQ/it/TFwFS4bbTV0= 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:mime-version:content-transfer-encoding; bh=eJ8w8RlGTgKwSve8kq67XCo8awUqV+HYghFRfD7mOnw=; b=k1mu+xGsUKcyGy5sy0J3/NsRhzOkoFR4QOmbMjgSCrvGRH3wcrmrAj2F+qHPaZudX8 PBTX7WuRcWs9PbWNfNhRk0FOQXZnRo/Pf6BZNe+XAjbwZ0mrs38Z9gGJ6p7nYO+qaVLh NN4XunuEoF9MhVR0vlCUPEshe1YvJtpgIfSsnYrUEE2PcrTukqvx7ky38twR2oYas6Uv bZ81hWkfIME8aU3x0UES19u1cYW7Q8gNa+9gfWc23OEW6s6bSiPE5ZW4rzpIDndrZnX+ w7ZM2HhoKSoeMkmaDHZi0cE7urtoElmjZRDDOQVn/B5fdWGu1w+DsgpW99iNGu7mPBqK BOUA== X-Gm-Message-State: AOAM532tWQu6LV6c0gQ0Lq47wRRO5tvpMVw6FStTvvwtpv6C8f6cOZIc 3oThUl/tnG2Y80B455OiD9zukg== X-Google-Smtp-Source: ABdhPJzfVk7O7EvLgEU3fkwifOsBFdREevvolrDbGInAARblPmWxv5XpCBmdblaIrL4B0RjyWF9I0g== X-Received: by 2002:a65:6916:: with SMTP id s22mr816271pgq.128.1592431412861; Wed, 17 Jun 2020 15:03:32 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id 9sm759447pfu.181.2020.06.17.15.03.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:30 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Matt Denton , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 2/7] fs: Move __scm_install_fd() to __fd_install_received() Date: Wed, 17 Jun 2020 15:03:22 -0700 Message-Id: <20200617220327.3731559-3-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org In preparation for users of the "install a received file" logic outside of net/ (pidfd and seccomp), relocate and rename __scm_install_fd() from net/core/scm.c to __fd_install_received() in fs/file.c, and provide a wrapper named fd_install_received_user(), as future patches will change the interface to __fd_install_received(). Signed-off-by: Kees Cook --- fs/file.c | 45 ++++++++++++++++++++++++++++++++++++++++++++ include/linux/file.h | 8 ++++++++ include/net/scm.h | 1 - net/compat.c | 2 +- net/core/scm.c | 32 +------------------------------ 5 files changed, 55 insertions(+), 33 deletions(-) diff --git a/fs/file.c b/fs/file.c index abb8b7081d7a..f2167d6feec6 100644 --- a/fs/file.c +++ b/fs/file.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -18,6 +19,8 @@ #include #include #include +#include +#include unsigned int sysctl_nr_open __read_mostly = 1024*1024; unsigned int sysctl_nr_open_min = BITS_PER_LONG; @@ -931,6 +934,48 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) return err; } +/** + * __fd_install_received() - Install received file into file descriptor table + * + * @file: struct file that was received from another process + * @ufd: __user pointer to write new fd number to + * @o_flags: the O_* flags to apply to the new fd entry + * + * Installs a received file into the file descriptor table, with appropriate + * checks and count updates. Writes the fd number to userspace. + * + * Returns -ve on error. + */ +int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_flags) +{ + struct socket *sock; + int new_fd; + int error; + + error = security_file_receive(file); + if (error) + return error; + + new_fd = get_unused_fd_flags(o_flags); + if (new_fd < 0) + return new_fd; + + error = put_user(new_fd, ufd); + if (error) { + put_unused_fd(new_fd); + return error; + } + + /* Bump the usage count and install the file. */ + sock = sock_from_file(file, &error); + if (sock) { + sock_update_netprioidx(&sock->sk->sk_cgrp_data); + sock_update_classid(&sock->sk->sk_cgrp_data); + } + fd_install(new_fd, get_file(file)); + return 0; +} + static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags) { int err = -EBADF; diff --git a/include/linux/file.h b/include/linux/file.h index 122f80084a3e..fe18a1a0d555 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -91,6 +91,14 @@ extern void put_unused_fd(unsigned int fd); extern void fd_install(unsigned int fd, struct file *file); +extern int __fd_install_received(struct file *file, int __user *ufd, + unsigned int o_flags); +static inline int fd_install_received_user(struct file *file, int __user *ufd, + unsigned int o_flags) +{ + return __fd_install_received(file, ufd, o_flags); +} + extern void flush_delayed_fput(void); extern void __fput_sync(struct file *); diff --git a/include/net/scm.h b/include/net/scm.h index 581a94d6c613..1ce365f4c256 100644 --- a/include/net/scm.h +++ b/include/net/scm.h @@ -37,7 +37,6 @@ struct scm_cookie { #endif }; -int __scm_install_fd(struct file *file, int __user *ufd, unsigned int o_flags); void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm); void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm); int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm); diff --git a/net/compat.c b/net/compat.c index 27d477fdcaa0..94f288e8dac5 100644 --- a/net/compat.c +++ b/net/compat.c @@ -298,7 +298,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) int err = 0, i; for (i = 0; i < fdmax; i++) { - err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); + err = fd_install_received_user(scm->fp->fp[i], cmsg_data + i, o_flags); if (err) break; } diff --git a/net/core/scm.c b/net/core/scm.c index 6151678c73ed..df190f1fdd28 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -280,36 +280,6 @@ void put_cmsg_scm_timestamping(struct msghdr *msg, struct scm_timestamping_inter } EXPORT_SYMBOL(put_cmsg_scm_timestamping); -int __scm_install_fd(struct file *file, int __user *ufd, unsigned int o_flags) -{ - struct socket *sock; - int new_fd; - int error; - - error = security_file_receive(file); - if (error) - return error; - - new_fd = get_unused_fd_flags(o_flags); - if (new_fd < 0) - return new_fd; - - error = put_user(new_fd, ufd); - if (error) { - put_unused_fd(new_fd); - return error; - } - - /* Bump the usage count and install the file. */ - sock = sock_from_file(file, &error); - if (sock) { - sock_update_netprioidx(&sock->sk->sk_cgrp_data); - sock_update_classid(&sock->sk->sk_cgrp_data); - } - fd_install(new_fd, get_file(file)); - return 0; -} - static int scm_max_fds(struct msghdr *msg) { if (msg->msg_controllen <= sizeof(struct cmsghdr)) @@ -336,7 +306,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) } for (i = 0; i < fdmax; i++) { - err = __scm_install_fd(scm->fp->fp[i], cmsg_data + i, o_flags); + err = fd_install_received_user(scm->fp->fp[i], cmsg_data + i, o_flags); if (err) break; } From patchwork Wed Jun 17 22:03:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610807 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 2F7B390 for ; Wed, 17 Jun 2020 22:04:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 122BF2070A for ; Wed, 17 Jun 2020 22:04:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="CyK2Y7jK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726758AbgFQWEl (ORCPT ); Wed, 17 Jun 2020 18:04:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44912 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727024AbgFQWDd (ORCPT ); Wed, 17 Jun 2020 18:03:33 -0400 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A707CC0613EE for ; Wed, 17 Jun 2020 15:03:32 -0700 (PDT) Received: by mail-pl1-x644.google.com with SMTP id n2so1545263pld.13 for ; Wed, 17 Jun 2020 15:03:32 -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 :mime-version:content-transfer-encoding; bh=JfpI1GG/4ASH8XTwm9WwhsnqJPuQkeJ3KfU5iE0siPk=; b=CyK2Y7jKcg3jWb83XP40NF0XUYGVXD+1V+4pT7KZSo4wCLqQ9iB9bRiDEd4nW56PXM hpIQOB7drdivotPmqckm3icsG3S6fJqxUXZUgTWB/JGpBy0Mts6JHOvn0vGN1qiSJx8E A8747kbFVyLgnx3F2OQvdMh3YUSLu0KYMhp+0= 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:mime-version:content-transfer-encoding; bh=JfpI1GG/4ASH8XTwm9WwhsnqJPuQkeJ3KfU5iE0siPk=; b=Zcl4qQxUy7kntZgiG6v2nbLotIAY0OBTN53MQHeT+VLw6mw48nO3Ifs5CQFX4CtJF1 lEWMMCA7kK5k0beVhTnUAJRQxO5+9j04BHVaztCcnNRWTmHbb6RDTinsHac9MWRPoYBm 6/o5tzGwxc7y3ILbTwEVZ2tLdXyFr0kkpAwuEVOsxs8KLfL1qsLIi8MjXkxLHhFP4myb ODub4KLWSk3Zf1IhoyiI0GzqBg7PTd7IjM6EceHuAgdH2ux2IRRXPxe1HfUBTzvpD0B+ oTu0N/8pwgrhcVeDKQMnjC53mvxB/4PyRBzHAdPxG5K1uTnmQwRGOTm5f4/tDRPrVd8l MvXg== X-Gm-Message-State: AOAM533VTBqrNHttyoFu799kUcqe6T7yu6199T5Md7Yb/H9DN/BJGeA8 zi0+tqwLfbzSa8F3Dt8qpxcGOA== X-Google-Smtp-Source: ABdhPJyaT2WGGTn+xIpi/7pv38Ikw5LMtpji1hqM1t34IkAV/SXn2PXvoXUf9SlukpLNRSDtc3ssEA== X-Received: by 2002:a17:90a:d803:: with SMTP id a3mr1130991pjv.125.1592431412195; Wed, 17 Jun 2020 15:03:32 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id t201sm755132pfc.104.2020.06.17.15.03.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:30 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Matt Denton , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 3/7] fs: Add fd_install_received() wrapper for __fd_install_received() Date: Wed, 17 Jun 2020 15:03:23 -0700 Message-Id: <20200617220327.3731559-4-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org For both pidfd and seccomp, the __user pointer is not used. Update __fd_install_received() to make writing to ufd optional via a NULL check. However, for the fd_install_received_user() wrapper, ufd is NULL checked so an -EFAULT can be returned to avoid changing the SCM_RIGHTS interface behavior. Add new wrapper fd_install_received() for pidfd and seccomp that does not use the ufd argument. For the new helper, the new fd needs to be returned on success. Update the existing callers to handle it. Signed-off-by: Kees Cook Reviewed-by: Sargun Dhillon --- fs/file.c | 22 ++++++++++++++-------- include/linux/file.h | 7 +++++++ net/compat.c | 2 +- net/core/scm.c | 2 +- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/fs/file.c b/fs/file.c index f2167d6feec6..de85a42defe2 100644 --- a/fs/file.c +++ b/fs/file.c @@ -942,9 +942,10 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) * @o_flags: the O_* flags to apply to the new fd entry * * Installs a received file into the file descriptor table, with appropriate - * checks and count updates. Writes the fd number to userspace. + * checks and count updates. Optionally writes the fd number to userspace, if + * @ufd is non-NULL. * - * Returns -ve on error. + * Returns newly install fd or -ve on error. */ int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_flags) { @@ -960,20 +961,25 @@ int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_fla if (new_fd < 0) return new_fd; - error = put_user(new_fd, ufd); - if (error) { - put_unused_fd(new_fd); - return error; + if (ufd) { + error = put_user(new_fd, ufd); + if (error) { + put_unused_fd(new_fd); + return error; + } } - /* Bump the usage count and install the file. */ + /* Bump the usage count and install the file. The resulting value of + * "error" is ignored here since we only need to take action when + * the file is a socket and testing "sock" for NULL is sufficient. + */ sock = sock_from_file(file, &error); if (sock) { sock_update_netprioidx(&sock->sk->sk_cgrp_data); sock_update_classid(&sock->sk->sk_cgrp_data); } fd_install(new_fd, get_file(file)); - return 0; + return new_fd; } static int ksys_dup3(unsigned int oldfd, unsigned int newfd, int flags) diff --git a/include/linux/file.h b/include/linux/file.h index fe18a1a0d555..e19974ed9322 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -9,6 +9,7 @@ #include #include #include +#include struct file; @@ -96,8 +97,14 @@ extern int __fd_install_received(struct file *file, int __user *ufd, static inline int fd_install_received_user(struct file *file, int __user *ufd, unsigned int o_flags) { + if (ufd == NULL) + return -EFAULT; return __fd_install_received(file, ufd, o_flags); } +static inline int fd_install_received(struct file *file, unsigned int o_flags) +{ + return __fd_install_received(file, NULL, o_flags); +} extern void flush_delayed_fput(void); extern void __fput_sync(struct file *); diff --git a/net/compat.c b/net/compat.c index 94f288e8dac5..71494337cca7 100644 --- a/net/compat.c +++ b/net/compat.c @@ -299,7 +299,7 @@ void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm) for (i = 0; i < fdmax; i++) { err = fd_install_received_user(scm->fp->fp[i], cmsg_data + i, o_flags); - if (err) + if (err < 0) break; } diff --git a/net/core/scm.c b/net/core/scm.c index df190f1fdd28..b9a0442ebd26 100644 --- a/net/core/scm.c +++ b/net/core/scm.c @@ -307,7 +307,7 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm) for (i = 0; i < fdmax; i++) { err = fd_install_received_user(scm->fp->fp[i], cmsg_data + i, o_flags); - if (err) + if (err < 0) break; } From patchwork Wed Jun 17 22:03:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610781 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 27310913 for ; Wed, 17 Jun 2020 22:03:33 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 0E87221852 for ; Wed, 17 Jun 2020 22:03:32 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="AssCNT+f" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727065AbgFQWDc (ORCPT ); Wed, 17 Jun 2020 18:03:32 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44902 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726761AbgFQWDc (ORCPT ); Wed, 17 Jun 2020 18:03:32 -0400 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0C348C06174E for ; Wed, 17 Jun 2020 15:03:32 -0700 (PDT) Received: by mail-pj1-x1041.google.com with SMTP id b7so2696010pju.0 for ; Wed, 17 Jun 2020 15:03:32 -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 :mime-version:content-transfer-encoding; bh=d7bw5q/TJxpqMicrHyIFon0t9gt+svhVPDuc7AUFnrI=; b=AssCNT+fl7FZXIZEydCky+FoEabK9WHSRC/Cz7t+GsLm7aO2UJ/YytOxHd1B0j1e+l XnLm1Rt+qfp2EUmGG/yFhjq91D26f7ku2eo0bUwGuEdGWnmwt6HPJOj85Snm44Eef3sC VVnAeX1Tp5FyYfkxWhsd8A7EzPvh9OS4GeLdE= 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:mime-version:content-transfer-encoding; bh=d7bw5q/TJxpqMicrHyIFon0t9gt+svhVPDuc7AUFnrI=; b=kQv52QJsY60pBwXFUEWmrhW8vLy0fG/ni+L7L8V7ggYfGUirkhqok/V9sleWHKLpmk fekFeeK1JAwOVQPBLNfQc0Vs/YOHXuY4VgHuy571Fe7xgKa5a73WuzlF3jI902U2+gIM gilYcOU5UvuqZlep+FWKd08HxaZJXB9lkDkHdHClyN8uobXXrQ5mJ9TIiBK1duKjW/TU qoSKEf7GVhQi82rl9eqIgCHQpfAj4ybV/ye5WebjFujc2XXOJjXvbNisCjln/I0ANxqB U+enDQGvRcbFYIh51oeJrMszTVON7rJISxjTffgLEMiMWbXByfkpUVnvGs0PRFD7/Rk3 e/Rw== X-Gm-Message-State: AOAM5337luZgbgeWIkyaKN0fVrPBYJ6GoB5KTmxnrZAyi+o2CH5Gexp2 QIuw3y/g16+ixZQkx0RQJCX0Hw== X-Google-Smtp-Source: ABdhPJy8S/6IprS//E4loSZ1ZgKk4AjU+iw9QdwwDUrpFc38iJU+1J4kMZ7xNS+1rFNTzrhm1LjJ3w== X-Received: by 2002:a17:90a:f3c4:: with SMTP id ha4mr1155509pjb.18.1592431411577; Wed, 17 Jun 2020 15:03:31 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id n24sm461968pjt.47.2020.06.17.15.03.30 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:30 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Matt Denton , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 4/7] pidfd: Replace open-coded partial fd_install_received() Date: Wed, 17 Jun 2020 15:03:24 -0700 Message-Id: <20200617220327.3731559-5-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org The sock counting (sock_update_netprioidx() and sock_update_classid()) was missing from pidfd's implementation of received fd installation. Replace the open-coded version with a call to the new fd_install_received() helper. Fixes: 8649c322f75c ("pid: Implement pidfd_getfd syscall") Signed-off-by: Kees Cook --- kernel/pid.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/kernel/pid.c b/kernel/pid.c index f1496b757162..24924ec5df0e 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -635,18 +635,9 @@ static int pidfd_getfd(struct pid *pid, int fd) if (IS_ERR(file)) return PTR_ERR(file); - ret = security_file_receive(file); - if (ret) { - fput(file); - return ret; - } - - ret = get_unused_fd_flags(O_CLOEXEC); + ret = fd_install_received(file, O_CLOEXEC); if (ret < 0) fput(file); - else - fd_install(ret, file); - return ret; } From patchwork Wed Jun 17 22:03:25 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610793 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 3D515913 for ; Wed, 17 Jun 2020 22:04:06 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 23CC421548 for ; Wed, 17 Jun 2020 22:04:06 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="c99QlzL+" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726835AbgFQWEF (ORCPT ); Wed, 17 Jun 2020 18:04:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44908 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727774AbgFQWDj (ORCPT ); Wed, 17 Jun 2020 18:03:39 -0400 Received: from mail-pl1-x642.google.com (mail-pl1-x642.google.com [IPv6:2607:f8b0:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9D25C0617BD for ; Wed, 17 Jun 2020 15:03:36 -0700 (PDT) Received: by mail-pl1-x642.google.com with SMTP id v24so1559609plo.6 for ; Wed, 17 Jun 2020 15:03:36 -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 :mime-version:content-transfer-encoding; bh=bMyqNAArtVNszcHrT7hbrHR3KGZeRc+Wdy+CDx/HSOE=; b=c99QlzL+vb52wHSMq2IhOXRkR899TunU/gm5RTSzYbT4UXIY6a0A3aI6Cz1MpXhAcK WMkTb7Crux0hfeow/zGeuszL+/t8eFGOKkXadEFHzSqPdbs++DEBvRaIKCqw7sP7dNmv gL2+KE6/YuTlFLgIY7Z/bm0fVIU+eJglNWXhc= 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:mime-version:content-transfer-encoding; bh=bMyqNAArtVNszcHrT7hbrHR3KGZeRc+Wdy+CDx/HSOE=; b=qs/wnzbNjWyTdXX01uZLOh90rZxm2Yv626So8mIzJa2g7DaLcQsynDwJ3DuepOcApO anlc9f64Svo3d0Rp1KGoBYHKt7seZ5BwPwarhXfGrtdIwqF+ghY4E7ZMDv4GD4u/Vhyd y0nV2CRB7UEGKPkgflJbqFudfTHv/lPCeStQnLPzB8YwG6RBFL1JL4GAgXWlr7C2A0wb WZuxgp4+mJ40zYG6Z7lrsqehQiRZ7/+Ih+y1YC8iKSAPZj0QLs6JIosowBxFen22HL3p 0H3L9M1K/YR5mi54GDWuW85eLf0LC9Rz3+nWTSSqSHWtVB12SaO3CV77ev1FRPtEv6Au +1oA== X-Gm-Message-State: AOAM532szGfKfPTUoiMg1oiADZ1lw2QCV0dajxaRaAbnb6H+XmoMgyNL 5h5KG23DBzL4bubzuf9AoT7B1Q== X-Google-Smtp-Source: ABdhPJyH669Vs7q6n5s9zYcdhTuLSHHg0GExur8GysnHPMjfY4l3Jea773ViskgyM9ZuqB0ob4lEiQ== X-Received: by 2002:a17:90a:22ad:: with SMTP id s42mr1039880pjc.200.1592431416440; Wed, 17 Jun 2020 15:03:36 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id s13sm784326pfc.136.2020.06.17.15.03.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:34 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Matt Denton , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 5/7] fs: Expand __fd_install_received() to accept fd Date: Wed, 17 Jun 2020 15:03:25 -0700 Message-Id: <20200617220327.3731559-6-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org Expand __fd_install_received() with support for replace_fd() for the coming seccomp "addfd" ioctl(). Add new wrapper fd_replace_received() for the new mode and update existing wrappers to retain old mode. Signed-off-by: Kees Cook --- fs/file.c | 22 +++++++++++++++++----- include/linux/file.h | 10 +++++++--- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/fs/file.c b/fs/file.c index de85a42defe2..9568bcfd1f44 100644 --- a/fs/file.c +++ b/fs/file.c @@ -937,6 +937,7 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) /** * __fd_install_received() - Install received file into file descriptor table * + * @fd: fd to install into (if negative, a new fd will be allocated) * @file: struct file that was received from another process * @ufd: __user pointer to write new fd number to * @o_flags: the O_* flags to apply to the new fd entry @@ -947,7 +948,8 @@ int replace_fd(unsigned fd, struct file *file, unsigned flags) * * Returns newly install fd or -ve on error. */ -int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_flags) +int __fd_install_received(int fd, struct file *file, int __user *ufd, + unsigned int o_flags) { struct socket *sock; int new_fd; @@ -957,9 +959,11 @@ int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_fla if (error) return error; - new_fd = get_unused_fd_flags(o_flags); - if (new_fd < 0) - return new_fd; + if (fd < 0) { + new_fd = get_unused_fd_flags(o_flags); + if (new_fd < 0) + return new_fd; + } if (ufd) { error = put_user(new_fd, ufd); @@ -969,6 +973,15 @@ int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_fla } } + if (fd < 0) + fd_install(new_fd, get_file(file)); + else { + new_fd = fd; + error = replace_fd(new_fd, file, o_flags); + if (error) + return error; + } + /* Bump the usage count and install the file. The resulting value of * "error" is ignored here since we only need to take action when * the file is a socket and testing "sock" for NULL is sufficient. @@ -978,7 +991,6 @@ int __fd_install_received(struct file *file, int __user *ufd, unsigned int o_fla sock_update_netprioidx(&sock->sk->sk_cgrp_data); sock_update_classid(&sock->sk->sk_cgrp_data); } - fd_install(new_fd, get_file(file)); return new_fd; } diff --git a/include/linux/file.h b/include/linux/file.h index e19974ed9322..04389b0da11b 100644 --- a/include/linux/file.h +++ b/include/linux/file.h @@ -92,18 +92,22 @@ extern void put_unused_fd(unsigned int fd); extern void fd_install(unsigned int fd, struct file *file); -extern int __fd_install_received(struct file *file, int __user *ufd, +extern int __fd_install_received(int fd, struct file *file, int __user *ufd, unsigned int o_flags); static inline int fd_install_received_user(struct file *file, int __user *ufd, unsigned int o_flags) { if (ufd == NULL) return -EFAULT; - return __fd_install_received(file, ufd, o_flags); + return __fd_install_received(-1, file, ufd, o_flags); } static inline int fd_install_received(struct file *file, unsigned int o_flags) { - return __fd_install_received(file, NULL, o_flags); + return __fd_install_received(-1, file, NULL, o_flags); +} +static inline int fd_replace_received(int fd, struct file *file, unsigned int o_flags) +{ + return __fd_install_received(fd, file, NULL, o_flags); } extern void flush_delayed_fput(void); From patchwork Wed Jun 17 22:03:26 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610785 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 167EB913 for ; Wed, 17 Jun 2020 22:03:48 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id E8B9021883 for ; Wed, 17 Jun 2020 22:03:47 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="C950usmg" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727812AbgFQWDq (ORCPT ); Wed, 17 Jun 2020 18:03:46 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44962 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727792AbgFQWDj (ORCPT ); Wed, 17 Jun 2020 18:03:39 -0400 Received: from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com [IPv6:2607:f8b0:4864:20::1043]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 046D5C06125C for ; Wed, 17 Jun 2020 15:03:38 -0700 (PDT) Received: by mail-pj1-x1043.google.com with SMTP id d6so1713969pjs.3 for ; Wed, 17 Jun 2020 15:03:38 -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 :mime-version:content-transfer-encoding; bh=og4kaRv0C+8yss5WIp58v59rEIm9Vwy5l2r507ap2AU=; b=C950usmgn5jd+JTm3LzV5vC7Q3WmRo17cJotatAjCi0ElLJMolZcQg4i1/g/I1LJVB sGcu6B8Dt6yN3jfwD+bB3ZFQR79/nehLCE4vM2Elc7hlKbwu14yksAaTNKw39pBYmg+o PReQDHtKGRbHWPsbgoEKiZ/2PyG9LFASF1qDM= 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:mime-version:content-transfer-encoding; bh=og4kaRv0C+8yss5WIp58v59rEIm9Vwy5l2r507ap2AU=; b=pSzjYoNYUakTYEJJKodObBS987z8UFdqvu6RmOtmuXICVTTZDOSf0zEp8ydPmy5ERw gmCh3lNDtJq16ecWKd+0kv8xiQCRBy75arzJlYYCBuHIApgzHjEP1xqdxZIwv3fs1P3a PcYkAg1Nw1IMaGsmKRWv7eMNfmRe5s+wx4nv3jiUcg8HdFvxbM/TEnm4fHEdRmRn0wCj CqS9smrsUi8WYS5yZoqWibdykmnW+yyoHRkJp+4oVvl8rhu8atwy//72zGSyS9txQVHp UZQ7530tylSNsEI/D2RcJ8AyrGPksn74qGXrnDJkeJgIcWTjbzRcVDbjIstADiSoJN/j wKuA== X-Gm-Message-State: AOAM531EUEa2HCZbqOGU7IwBlSAsb7XdA9WnzaWkGY8AP24r/uLzFk33 m0c8zcyx404cTXkU9xPHbRY5VQ== X-Google-Smtp-Source: ABdhPJzfFUvodKnHP5+bUjL1A5NhScqbSlP6OvOgEy3QlBf8vI8tc9eR4BAbwqA9S5HpZAzGhghOFg== X-Received: by 2002:a17:902:d201:: with SMTP id t1mr1005134ply.327.1592431417399; Wed, 17 Jun 2020 15:03:37 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id r4sm691679pgp.60.2020.06.17.15.03.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:34 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Matt Denton , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 6/7] seccomp: Introduce addfd ioctl to seccomp user notifier Date: Wed, 17 Jun 2020 15:03:26 -0700 Message-Id: <20200617220327.3731559-7-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org From: Sargun Dhillon This adds a seccomp notifier ioctl which allows for the listener to "add" file descriptors to a process which originated a seccomp user notification. This allows calls like mount, and mknod to be "implemented", as the return value, and the arguments are data in memory. On the other hand, calls like connect can be "implemented" using pidfd_getfd. Unfortunately, there are calls which return file descriptors, like open, which are vulnerable to ToCToU attacks, and require that the more privileged supervisor can inspect the argument, and perform the syscall on behalf of the process generating the notification. This allows the file descriptor generated from that open call to be returned to the calling process. In addition, there is functionality to allow for replacement of specific file descriptors, following dup2-like semantics. The ioctl handling is based on the discussions[1] of how Extensible Arguments should interact with ioctls. Instead of building size into the addfd structure, make it a function of the ioctl command (which is how sizes are normally passed to ioctls). To support forward and backward compatibility, just mask out the direction and size, and match everything. The size (and any future direction) checks are done along with copy_struct_from_user() logic. As a note, the seccomp_notif_addfd structure is laid out based on 8-byte alignment without requiring packing as there have been packing issues with uapi highlighted before[1][2]. Although we could overload the newfd field and use -1 to indicate that it is not to be used, doing so requires changing the size of the fd field, and introduces struct packing complexity. [1]: https://lore.kernel.org/lkml/87o8w9bcaf.fsf@mid.deneb.enyo.de/ [2]: https://lore.kernel.org/lkml/a328b91d-fd8f-4f27-b3c2-91a9c45f18c0@rasmusvillemoes.dk/ [3]: https://lore.kernel.org/lkml/20200612104629.GA15814@ircssh-2.c.rugged-nimbus-611.internal Suggested-by: Matt Denton Link: https://lore.kernel.org/r/20200603011044.7972-4-sargun@sargun.me Signed-off-by: Sargun Dhillon Co-developed-by: Kees Cook Signed-off-by: Kees Cook --- include/uapi/linux/seccomp.h | 22 +++++ kernel/seccomp.c | 172 ++++++++++++++++++++++++++++++++++- 2 files changed, 193 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h index 965290f7dcc2..6ba18b82a02e 100644 --- a/include/uapi/linux/seccomp.h +++ b/include/uapi/linux/seccomp.h @@ -113,6 +113,25 @@ struct seccomp_notif_resp { __u32 flags; }; +/* valid flags for seccomp_notif_addfd */ +#define SECCOMP_ADDFD_FLAG_SETFD (1UL << 0) /* Specify remote fd */ + +/** + * struct seccomp_notif_addfd + * @id: The ID of the seccomp notification + * @flags: SECCOMP_ADDFD_FLAG_* + * @srcfd: The local fd number + * @newfd: Optional remote FD number if SETFD option is set, otherwise 0. + * @newfd_flags: The O_* flags the remote FD should have applied + */ +struct seccomp_notif_addfd { + __u64 id; + __u32 flags; + __u32 srcfd; + __u32 newfd; + __u32 newfd_flags; +}; + #define SECCOMP_IOC_MAGIC '!' #define SECCOMP_IO(nr) _IO(SECCOMP_IOC_MAGIC, nr) #define SECCOMP_IOR(nr, type) _IOR(SECCOMP_IOC_MAGIC, nr, type) @@ -124,5 +143,8 @@ struct seccomp_notif_resp { #define SECCOMP_IOCTL_NOTIF_SEND SECCOMP_IOWR(1, \ struct seccomp_notif_resp) #define SECCOMP_IOCTL_NOTIF_ID_VALID SECCOMP_IOW(2, __u64) +/* On success, the return value is the remote process's added fd number */ +#define SECCOMP_IOCTL_NOTIF_ADDFD SECCOMP_IOW(3, \ + struct seccomp_notif_addfd) #endif /* _UAPI_LINUX_SECCOMP_H */ diff --git a/kernel/seccomp.c b/kernel/seccomp.c index 0ed57e8c49d0..a7c20c408b83 100644 --- a/kernel/seccomp.c +++ b/kernel/seccomp.c @@ -87,10 +87,42 @@ struct seccomp_knotif { long val; u32 flags; - /* Signals when this has entered SECCOMP_NOTIFY_REPLIED */ + /* + * Signals when this has changed states, such as the listener + * dying, a new seccomp addfd message, or changing to REPLIED + */ struct completion ready; struct list_head list; + + /* outstanding addfd requests */ + struct list_head addfd; +}; + +/** + * struct seccomp_kaddfd - container for seccomp_addfd ioctl messages + * + * @file: A reference to the file to install in the other task + * @fd: The fd number to install it at. If the fd number is -1, it means the + * installing process should allocate the fd as normal. + * @flags: The flags for the new file descriptor. At the moment, only O_CLOEXEC + * is allowed. + * @ret: The return value of the installing process. It is set to the fd num + * upon success (>= 0). + * @completion: Indicates that the installing process has completed fd + * installation, or gone away (either due to successful + * reply, or signal) + * + */ +struct seccomp_kaddfd { + struct file *file; + int fd; + unsigned int flags; + + /* To only be set on reply */ + int ret; + struct completion completion; + struct list_head list; }; /** @@ -793,6 +825,17 @@ static u64 seccomp_next_notify_id(struct seccomp_filter *filter) return filter->notif->next_id++; } +static void seccomp_handle_addfd(struct seccomp_kaddfd *addfd) +{ + /* + * Remove the notification, and reset the list pointers, indicating + * that it has been handled. + */ + list_del_init(&addfd->list); + addfd->ret = fd_replace_received(addfd->fd, addfd->file, addfd->flags); + complete(&addfd->completion); +} + static int seccomp_do_user_notification(int this_syscall, struct seccomp_filter *match, const struct seccomp_data *sd) @@ -801,6 +844,7 @@ static int seccomp_do_user_notification(int this_syscall, u32 flags = 0; long ret = 0; struct seccomp_knotif n = {}; + struct seccomp_kaddfd *addfd, *tmp; mutex_lock(&match->notify_lock); err = -ENOSYS; @@ -813,6 +857,7 @@ static int seccomp_do_user_notification(int this_syscall, n.id = seccomp_next_notify_id(match); init_completion(&n.ready); list_add(&n.list, &match->notif->notifications); + INIT_LIST_HEAD(&n.addfd); up(&match->notif->request); wake_up_poll(&match->wqh, EPOLLIN | EPOLLRDNORM); @@ -821,14 +866,31 @@ static int seccomp_do_user_notification(int this_syscall, /* * This is where we wait for a reply from userspace. */ +wait: err = wait_for_completion_interruptible(&n.ready); mutex_lock(&match->notify_lock); if (err == 0) { + /* Check if we were woken up by a addfd message */ + addfd = list_first_entry_or_null(&n.addfd, + struct seccomp_kaddfd, list); + if (addfd && n.state != SECCOMP_NOTIFY_REPLIED) { + seccomp_handle_addfd(addfd); + mutex_unlock(&match->notify_lock); + goto wait; + } ret = n.val; err = n.error; flags = n.flags; } + /* If there were any pending addfd calls, clear them out */ + list_for_each_entry_safe(addfd, tmp, &n.addfd, list) { + /* The process went away before we got a chance to handle it */ + addfd->ret = -ESRCH; + list_del_init(&addfd->list); + complete(&addfd->completion); + } + /* * Note that it's possible the listener died in between the time when * we were notified of a respons (or a signal) and when we were able to @@ -1069,6 +1131,11 @@ static int seccomp_notify_release(struct inode *inode, struct file *file) knotif->error = -ENOSYS; knotif->val = 0; + /* + * We do not need to wake up any pending addfd messages, as + * the notifier will do that for us, as this just looks + * like a standard reply. + */ complete(&knotif->ready); } @@ -1233,12 +1300,107 @@ static long seccomp_notify_id_valid(struct seccomp_filter *filter, return ret; } +static long seccomp_notify_addfd(struct seccomp_filter *filter, + struct seccomp_notif_addfd __user *uaddfd, + unsigned int size) +{ + struct seccomp_notif_addfd addfd; + struct seccomp_knotif *knotif; + struct seccomp_kaddfd kaddfd; + int ret; + + /* 24 is original sizeof(struct seccomp_notif_addfd) */ + if (size < 24 || size >= PAGE_SIZE) + return -EINVAL; + + ret = copy_struct_from_user(&addfd, sizeof(addfd), uaddfd, size); + if (ret) + return ret; + + if (addfd.newfd_flags & ~O_CLOEXEC) + return -EINVAL; + + if (addfd.flags & ~SECCOMP_ADDFD_FLAG_SETFD) + return -EINVAL; + + if (addfd.newfd && !(addfd.flags & SECCOMP_ADDFD_FLAG_SETFD)) + return -EINVAL; + + kaddfd.file = fget(addfd.srcfd); + if (!kaddfd.file) + return -EBADF; + + kaddfd.flags = addfd.newfd_flags; + kaddfd.fd = (addfd.flags & SECCOMP_ADDFD_FLAG_SETFD) ? + addfd.newfd : -1; + init_completion(&kaddfd.completion); + + ret = mutex_lock_interruptible(&filter->notify_lock); + if (ret < 0) + goto out; + + knotif = find_notification(filter, addfd.id); + if (!knotif) { + ret = -ENOENT; + goto out_unlock; + } + + /* + * We do not want to allow for FD injection to occur before the + * notification has been picked up by a userspace handler, or after + * the notification has been replied to. + */ + if (knotif->state != SECCOMP_NOTIFY_SENT) { + ret = -EINPROGRESS; + goto out_unlock; + } + + list_add(&kaddfd.list, &knotif->addfd); + complete(&knotif->ready); + mutex_unlock(&filter->notify_lock); + + /* Now we wait for it to be processed or be interrupted */ + ret = wait_for_completion_interruptible(&kaddfd.completion); + if (ret == 0) { + /* + * We had a successful completion. The other side has already + * removed us from the addfd queue, and + * wait_for_completion_interruptible has a memory barrier upon + * success that lets us read this value directly without + * locking. + */ + ret = kaddfd.ret; + goto out; + } + + mutex_lock(&filter->notify_lock); + /* + * Even though we were woken up by a signal and not a successful + * completion, a completion may have happened in the mean time. + * + * We need to check again if the addfd request has been handled, + * and if not, we will remove it from the queue. + */ + if (list_empty(&kaddfd.list)) + ret = kaddfd.ret; + else + list_del(&kaddfd.list); + +out_unlock: + mutex_unlock(&filter->notify_lock); +out: + fput(kaddfd.file); + + return ret; +} + static long seccomp_notify_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { struct seccomp_filter *filter = file->private_data; void __user *buf = (void __user *)arg; + /* Fixed-size ioctls */ switch (cmd) { case SECCOMP_IOCTL_NOTIF_RECV: return seccomp_notify_recv(filter, buf); @@ -1247,9 +1409,17 @@ static long seccomp_notify_ioctl(struct file *file, unsigned int cmd, case SECCOMP_IOCTL_NOTIF_ID_VALID_WRONG_DIR: case SECCOMP_IOCTL_NOTIF_ID_VALID: return seccomp_notify_id_valid(filter, buf); + } + + /* Extensible Argument ioctls */ +#define EA_IOCTL(cmd) ((cmd) & ~(IOC_INOUT | IOCSIZE_MASK)) + switch (EA_IOCTL(cmd)) { + case EA_IOCTL(SECCOMP_IOCTL_NOTIF_ADDFD): + return seccomp_notify_addfd(filter, buf, _IOC_SIZE(cmd)); default: return -EINVAL; } +#undef EA_IOCTL } static __poll_t seccomp_notify_poll(struct file *file, From patchwork Wed Jun 17 22:03:27 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kees Cook X-Patchwork-Id: 11610795 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 6CC0990 for ; Wed, 17 Jun 2020 22:04:15 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 47D4821897 for ; Wed, 17 Jun 2020 22:04:15 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="THJBRTFw" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727030AbgFQWEG (ORCPT ); Wed, 17 Jun 2020 18:04:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:44946 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727037AbgFQWDg (ORCPT ); Wed, 17 Jun 2020 18:03:36 -0400 Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5D736C0617B9 for ; Wed, 17 Jun 2020 15:03:36 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id j1so1810064pfe.4 for ; Wed, 17 Jun 2020 15:03:36 -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 :mime-version:content-transfer-encoding; bh=s6nxy7AwZGtoql+LbUkBHllnwgWYj58mW1sDysdJ1vg=; b=THJBRTFwPqKrgTynwdqc6tP63XYizHMXDJ/tpFigmCgXKT9ywN0gTaISfScTuCIoXX ELo2H8FGafq7E4i3aK7qj7GodIAF0JTxZI1VuLgPj8LasDHq26PRD5qXlgn9cfw3XycB bTYPGsFKBTK0Nfzl/tNhQLb7qAEbwTILjsSUk= 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:mime-version:content-transfer-encoding; bh=s6nxy7AwZGtoql+LbUkBHllnwgWYj58mW1sDysdJ1vg=; b=oTSNTr/zFUr5vepycznGzaiy6tGbLNKwglFY3mcX07lQf7VVSSr2le6mivhnzhBCXK Auf0oVhLXI9rk/gY1TC3krGK7esOnuLQaJ2AU2s/BG6sBtgHubm8F/Z/aGdgN4ZV91zA wAIz0DtwMk1F7q+y4FSdoq8hVIHeTlY5QPolQJH2UrO8kY4lY+SNZt+6d9qfrc8gVbX9 cA42+ugYW2hI/JSAAfCOh8kR6vJz/g44knxxF9iykVm5RRmKFFm/AbXgeNo6TiuPbe+j wj56yhD9uaaXNQVKEle+p1woahWS+kVRlAVaCWdqpCX6bhQtwqV8DuV7JuHUgfnChD3C YwLw== X-Gm-Message-State: AOAM530uy0o9fOtY8BXlzPNgU8HAv2bJEhyiXvXF73qs4y1Co2+KDKmq ecZLI4h8/f8N2Q+OtwkhdY5xQA== X-Google-Smtp-Source: ABdhPJxS49EBdf4oJlb/sifNaCdN+lj0qN/PquPppLJoOyakXqU7DF9KsQu4IE6nNJB5iuBJQB4ZyQ== X-Received: by 2002:a63:205b:: with SMTP id r27mr770691pgm.326.1592431415799; Wed, 17 Jun 2020 15:03:35 -0700 (PDT) Received: from www.outflux.net (smtp.outflux.net. [198.145.64.163]) by smtp.gmail.com with ESMTPSA id d22sm682059pgh.64.2020.06.17.15.03.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 17 Jun 2020 15:03:34 -0700 (PDT) From: Kees Cook To: linux-kernel@vger.kernel.org Cc: Kees Cook , Sargun Dhillon , Christian Brauner , Tycho Andersen , David Laight , Christoph Hellwig , "David S. Miller" , Jakub Kicinski , Alexander Viro , Aleksa Sarai , Matt Denton , Jann Horn , Chris Palmer , Robert Sesek , Giuseppe Scrivano , Greg Kroah-Hartman , Andy Lutomirski , Will Drewry , Shuah Khan , netdev@vger.kernel.org, containers@lists.linux-foundation.org, linux-api@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: [PATCH v5 7/7] selftests/seccomp: Test SECCOMP_IOCTL_NOTIF_ADDFD Date: Wed, 17 Jun 2020 15:03:27 -0700 Message-Id: <20200617220327.3731559-8-keescook@chromium.org> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20200617220327.3731559-1-keescook@chromium.org> References: <20200617220327.3731559-1-keescook@chromium.org> MIME-Version: 1.0 Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org From: Sargun Dhillon Test whether we can add file descriptors in response to notifications. This injects the file descriptors via notifications, and then uses kcmp to determine whether or not it has been successful. It also includes some basic sanity checking for arguments. Signed-off-by: Sargun Dhillon Link: https://lore.kernel.org/r/20200603011044.7972-5-sargun@sargun.me Co-developed-by: Kees Cook Signed-off-by: Kees Cook --- tools/testing/selftests/seccomp/seccomp_bpf.c | 229 ++++++++++++++++++ 1 file changed, 229 insertions(+) diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index 4e1891f8a0cd..143eafdc4fdc 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -45,6 +45,7 @@ #include #include #include +#include #include #include @@ -168,7 +169,9 @@ struct seccomp_metadata { #ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER #define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3) +#endif +#ifndef SECCOMP_RET_USER_NOTIF #define SECCOMP_RET_USER_NOTIF 0x7fc00000U #define SECCOMP_IOC_MAGIC '!' @@ -204,6 +207,39 @@ struct seccomp_notif_sizes { }; #endif +#ifndef SECCOMP_IOCTL_NOTIF_ADDFD +/* On success, the return value is the remote process's added fd number */ +#define SECCOMP_IOCTL_NOTIF_ADDFD SECCOMP_IOW(3, \ + struct seccomp_notif_addfd) + +/* valid flags for seccomp_notif_addfd */ +#define SECCOMP_ADDFD_FLAG_SETFD (1UL << 0) /* Specify remote fd */ + +struct seccomp_notif_addfd { + __u64 id; + __u32 flags; + __u32 srcfd; + __u32 newfd; + __u32 newfd_flags; +}; +#endif + +struct seccomp_notif_addfd_small { + __u64 id; + char weird[4]; +}; +#define SECCOMP_IOCTL_NOTIF_ADDFD_SMALL \ + SECCOMP_IOW(3, struct seccomp_notif_addfd_small) + +struct seccomp_notif_addfd_big { + union { + struct seccomp_notif_addfd addfd; + char buf[sizeof(struct seccomp_notif_addfd) + 8]; + }; +}; +#define SECCOMP_IOCTL_NOTIF_ADDFD_BIG \ + SECCOMP_IOWR(3, struct seccomp_notif_addfd_big) + #ifndef PTRACE_EVENTMSG_SYSCALL_ENTRY #define PTRACE_EVENTMSG_SYSCALL_ENTRY 1 #define PTRACE_EVENTMSG_SYSCALL_EXIT 2 @@ -3833,6 +3869,199 @@ TEST(user_notification_filter_empty_threaded) EXPECT_GT((pollfd.revents & POLLHUP) ?: 0, 0); } +TEST(user_notification_addfd) +{ + pid_t pid; + long ret; + int status, listener, memfd, fd; + struct seccomp_notif_addfd addfd = {}; + struct seccomp_notif_addfd_small small = {}; + struct seccomp_notif_addfd_big big = {}; + struct seccomp_notif req = {}; + struct seccomp_notif_resp resp = {}; + /* 100 ms */ + struct timespec delay = { .tv_nsec = 100000000 }; + + memfd = memfd_create("test", 0); + ASSERT_GE(memfd, 0); + + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); + ASSERT_EQ(0, ret) { + TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); + } + + /* Check that the basic notification machinery works */ + listener = user_notif_syscall(__NR_getppid, + SECCOMP_FILTER_FLAG_NEW_LISTENER); + ASSERT_GE(listener, 0); + + pid = fork(); + ASSERT_GE(pid, 0); + + if (pid == 0) { + if (syscall(__NR_getppid) != USER_NOTIF_MAGIC) + exit(1); + exit(syscall(__NR_getppid) != USER_NOTIF_MAGIC); + } + + ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); + + addfd.srcfd = memfd; + addfd.newfd = 0; + addfd.id = req.id; + addfd.flags = 0x0; + + /* Verify bad newfd_flags cannot be set */ + addfd.newfd_flags = ~O_CLOEXEC; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); + EXPECT_EQ(errno, EINVAL); + addfd.newfd_flags = O_CLOEXEC; + + /* Verify bad flags cannot be set */ + addfd.flags = 0xff; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); + EXPECT_EQ(errno, EINVAL); + addfd.flags = 0; + + /* Verify that remote_fd cannot be set without setting flags */ + addfd.newfd = 1; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); + EXPECT_EQ(errno, EINVAL); + addfd.newfd = 0; + + /* Verify small size cannot be set */ + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD_SMALL, &small), -1); + EXPECT_EQ(errno, EINVAL); + + /* Verify we can't send bits filled in unknown buffer area */ + memset(&big, 0xAA, sizeof(big)); + big.addfd = addfd; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD_BIG, &big), -1); + EXPECT_EQ(errno, E2BIG); + + + /* Verify we can set an arbitrary remote fd */ + fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd); + /* + * The child has fds 0(stdin), 1(stdout), 2(stderr), 3(memfd), + * 4(listener), so the newly allocated fd should be 5. + */ + EXPECT_EQ(fd, 5); + EXPECT_EQ(filecmp(getpid(), pid, memfd, fd), 0); + + /* Verify we can set an arbitrary remote fd with large size */ + memset(&big, 0x0, sizeof(big)); + big.addfd = addfd; + fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD_BIG, &big); + EXPECT_EQ(fd, 6); + + /* Verify we can set a specific remote fd */ + addfd.newfd = 42; + addfd.flags = SECCOMP_ADDFD_FLAG_SETFD; + fd = ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd); + EXPECT_EQ(fd, 42); + EXPECT_EQ(filecmp(getpid(), pid, memfd, fd), 0); + + /* Resume syscall */ + resp.id = req.id; + resp.error = 0; + resp.val = USER_NOTIF_MAGIC; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0); + + /* + * This sets the ID of the ADD FD to the last request plus 1. The + * notification ID increments 1 per notification. + */ + addfd.id = req.id + 1; + + /* This spins until the underlying notification is generated */ + while (ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd) != -1 && + errno != -EINPROGRESS) + nanosleep(&delay, NULL); + + memset(&req, 0, sizeof(req)); + ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); + ASSERT_EQ(addfd.id, req.id); + + resp.id = req.id; + resp.error = 0; + resp.val = USER_NOTIF_MAGIC; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0); + + /* Wait for child to finish. */ + EXPECT_EQ(waitpid(pid, &status, 0), pid); + EXPECT_EQ(true, WIFEXITED(status)); + EXPECT_EQ(0, WEXITSTATUS(status)); + + close(memfd); +} + +TEST(user_notification_addfd_rlimit) +{ + pid_t pid; + long ret; + int status, listener, memfd; + struct seccomp_notif_addfd addfd = {}; + struct seccomp_notif req = {}; + struct seccomp_notif_resp resp = {}; + const struct rlimit lim = { + .rlim_cur = 0, + .rlim_max = 0, + }; + + memfd = memfd_create("test", 0); + ASSERT_GE(memfd, 0); + + ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); + ASSERT_EQ(0, ret) { + TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); + } + + /* Check that the basic notification machinery works */ + listener = user_notif_syscall(__NR_getppid, + SECCOMP_FILTER_FLAG_NEW_LISTENER); + ASSERT_GE(listener, 0); + + pid = fork(); + ASSERT_GE(pid, 0); + + if (pid == 0) + exit(syscall(__NR_getppid) != USER_NOTIF_MAGIC); + + + ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); + + ASSERT_EQ(prlimit(pid, RLIMIT_NOFILE, &lim, NULL), 0); + + addfd.srcfd = memfd; + addfd.newfd_flags = O_CLOEXEC; + addfd.newfd = 0; + addfd.id = req.id; + addfd.flags = 0; + + /* Should probably spot check /proc/sys/fs/file-nr */ + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); + EXPECT_EQ(errno, EMFILE); + + addfd.newfd = 100; + addfd.flags = SECCOMP_ADDFD_FLAG_SETFD; + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ADDFD, &addfd), -1); + EXPECT_EQ(errno, EBADF); + + resp.id = req.id; + resp.error = 0; + resp.val = USER_NOTIF_MAGIC; + + EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0); + + /* Wait for child to finish. */ + EXPECT_EQ(waitpid(pid, &status, 0), pid); + EXPECT_EQ(true, WIFEXITED(status)); + EXPECT_EQ(0, WEXITSTATUS(status)); + + close(memfd); +} + /* * TODO: * - expand NNP testing