From patchwork Fri Jun 24 23:51:52 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 9198259 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1343B6077D for ; Fri, 24 Jun 2016 23:52:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D9365284E1 for ; Fri, 24 Jun 2016 23:52:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B7215284E9; Fri, 24 Jun 2016 23:52:49 +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=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID 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 9408A284E1 for ; Fri, 24 Jun 2016 23:52:48 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751337AbcFXXwP (ORCPT ); Fri, 24 Jun 2016 19:52:15 -0400 Received: from mail-wm0-f53.google.com ([74.125.82.53]:36375 "EHLO mail-wm0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751080AbcFXXwO (ORCPT ); Fri, 24 Jun 2016 19:52:14 -0400 Received: by mail-wm0-f53.google.com with SMTP id f126so38516885wma.1 for ; Fri, 24 Jun 2016 16:52:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=An8gQvmDyTdjI7tWFqB2N2+MpOw76f8u8ZrlopRtFRo=; b=OkksiDOLIo67z/I+qt4biyouOh98Feswm65YpwCrdJEsD8Jq19XevHdp16HfQGLD+J 1tm66BUa/2f1Xl4SWt1EQGdbRjsefr77+rbOME5yHWjL0sG8+wit1lgFOi5BAMJHF4Av SeGsHcqsCF35Ze93RCFwNh7vVLj6zYodbshyT2QJq/oVrpbTyIqECPjqnKwnVRX6MXuW lmIlIhx1Oh07ctPo3kt+AmIifT7wBR22hbZTwB3NDWzn/hnXM886eJk20dLzSJaM+WMk RZavuUg17msmMLQQjaUJaWy1Z12XkKAGfzaq8a17rQwsuW8pRX5If6Q/JD3ZDh2hfrPs ZbGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=An8gQvmDyTdjI7tWFqB2N2+MpOw76f8u8ZrlopRtFRo=; b=PmTGXqZQoWkxwPGAV7IO6CuTSyFmYj+GCjMLhaKWR4Pr0L9GxS+ZXSM8OdcQoRESW1 BJP2JWUnLgm0NUkF9WnkAYVvrTbeVEzuBK2YB1rbreZaEUET6qfyxElvQ149tz5QGL2z PEEFefVAsyCktfWNKKNNz2DMjfeHDbhV5wb0JnE0IzMDdYzKNw1qv2J5qnhLSTUriqQc J+79BTCFmZN/mCBeqaHKDMA5ezsNMVckAWUJgDA5BWU4BYrhb8u2x1NAkIi7VDgNL7pW E+iCcHzkEpQmpddwjFdcwYSiE6I4sztR3W+YUq1We5+9LSSPUTfUkHZ1AcKpHVzeTfIf CgMw== X-Gm-Message-State: ALyK8tJyJ03Tp4oeegdm2igxnBrmlTiwruF9C29CGlN2ESxg6Q9p6oEmGiiSX9N4fZDXtSer X-Received: by 10.194.119.233 with SMTP id kx9mr6185742wjb.87.1466812332591; Fri, 24 Jun 2016 16:52:12 -0700 (PDT) Received: from jannh.zrh.corp.google.com ([100.105.131.44]) by smtp.gmail.com with ESMTPSA id t67sm672571wma.1.2016.06.24.16.52.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 24 Jun 2016 16:52:11 -0700 (PDT) From: Jann Horn To: Mike Marshall Cc: linux-kernel@vger.kernel.org, Eric Biederman , Alexander Viro , linux-fsdevel@vger.kernel.org, jann@thejh.net, Jann Horn Subject: [PATCH] orangefs: fix namespace handling Date: Sat, 25 Jun 2016 01:51:52 +0200 Message-Id: <1466812312-26093-1-git-send-email-jannh@google.com> X-Mailer: git-send-email 2.8.0.rc3.226.g39d4020 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In orangefs_inode_getxattr(), an fsuid is written to dmesg. The kuid is converted to a userspace uid via from_kuid(current_user_ns(), [...]), but since dmesg is global, init_user_ns should be used here instead. In copy_attributes_from_inode(), op_alloc() and fill_default_sys_attrs(), upcall structures are populated with uids/gids that have been mapped into the caller's namespace. However, those upcall structures are read by another process (the userspace filesystem driver), and that process might be running in another namespace. This effectively lets any user spoof its uid and gid as seen by the userspace filesystem driver. To fix the second issue, I just construct the opcall structures with init_user_ns uids/gids and require the filesystem server to run in the init namespace. Since orangefs is full of global state anyway (as the error message in DUMP_DEVICE_ERROR explains, there can only be one userspace orangefs filesystem driver at once), that shouldn't be a problem. [ Why does orangefs even exist in the kernel if everything does upcalls into userspace? What does orangefs do that couldn't be done with the FUSE interface? If there is no good answer to those questions, I'd prefer to see orangefs kicked out of the kernel. Can that be done for something that shipped in a release? According to commit f7ab093f74bf ("Orangefs: kernel client part 1"), they even already have a FUSE daemon, and the only rational reason (apart from "but most of our users report preferring to use our kernel module instead") given for not wanting to use FUSE is one "in-the-works" feature that could probably be integated into FUSE instead. ] This patch has been compile-tested. Signed-off-by: Jann Horn --- fs/orangefs/devorangefs-req.c | 7 +++++++ fs/orangefs/orangefs-cache.c | 4 ++-- fs/orangefs/orangefs-kernel.h | 4 ++-- fs/orangefs/orangefs-utils.c | 4 ++-- fs/orangefs/xattr.c | 4 ++-- 5 files changed, 15 insertions(+), 8 deletions(-) diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c index db170be..a287a66 100644 --- a/fs/orangefs/devorangefs-req.c +++ b/fs/orangefs/devorangefs-req.c @@ -116,6 +116,13 @@ static int orangefs_devreq_open(struct inode *inode, struct file *file) { int ret = -EINVAL; + /* in order to ensure that the filesystem driver sees correct UIDs */ + if (file->f_cred->user_ns != &init_user_ns) { + gossip_err("%s: device cannot be opened outside init_user_ns\n", + __func__); + goto out; + } + if (!(file->f_flags & O_NONBLOCK)) { gossip_err("%s: device cannot be opened in blocking mode\n", __func__); diff --git a/fs/orangefs/orangefs-cache.c b/fs/orangefs/orangefs-cache.c index 900a2e3..b6edbe9 100644 --- a/fs/orangefs/orangefs-cache.c +++ b/fs/orangefs/orangefs-cache.c @@ -136,10 +136,10 @@ struct orangefs_kernel_op_s *op_alloc(__s32 type) llu(new_op->tag), get_opname_string(new_op)); - new_op->upcall.uid = from_kuid(current_user_ns(), + new_op->upcall.uid = from_kuid(&init_user_ns, current_fsuid()); - new_op->upcall.gid = from_kgid(current_user_ns(), + new_op->upcall.gid = from_kgid(&init_user_ns, current_fsgid()); } else { gossip_err("op_alloc: kmem_cache_zalloc failed!\n"); diff --git a/fs/orangefs/orangefs-kernel.h b/fs/orangefs/orangefs-kernel.h index 2281882..a6834d4 100644 --- a/fs/orangefs/orangefs-kernel.h +++ b/fs/orangefs/orangefs-kernel.h @@ -600,8 +600,8 @@ int service_operation(struct orangefs_kernel_op_s *op, #define fill_default_sys_attrs(sys_attr, type, mode) \ do { \ - sys_attr.owner = from_kuid(current_user_ns(), current_fsuid()); \ - sys_attr.group = from_kgid(current_user_ns(), current_fsgid()); \ + sys_attr.owner = from_kuid(&init_user_ns, current_fsuid()); \ + sys_attr.group = from_kgid(&init_user_ns, current_fsgid()); \ sys_attr.perms = ORANGEFS_util_translate_mode(mode); \ sys_attr.mtime = 0; \ sys_attr.atime = 0; \ diff --git a/fs/orangefs/orangefs-utils.c b/fs/orangefs/orangefs-utils.c index 2d129b5..c5fbc62 100644 --- a/fs/orangefs/orangefs-utils.c +++ b/fs/orangefs/orangefs-utils.c @@ -153,12 +153,12 @@ static inline int copy_attributes_from_inode(struct inode *inode, */ attrs->mask = 0; if (iattr->ia_valid & ATTR_UID) { - attrs->owner = from_kuid(current_user_ns(), iattr->ia_uid); + attrs->owner = from_kuid(&init_user_ns, iattr->ia_uid); attrs->mask |= ORANGEFS_ATTR_SYS_UID; gossip_debug(GOSSIP_UTILS_DEBUG, "(UID) %d\n", attrs->owner); } if (iattr->ia_valid & ATTR_GID) { - attrs->group = from_kgid(current_user_ns(), iattr->ia_gid); + attrs->group = from_kgid(&init_user_ns, iattr->ia_gid); attrs->mask |= ORANGEFS_ATTR_SYS_GID; gossip_debug(GOSSIP_UTILS_DEBUG, "(GID) %d\n", attrs->group); } diff --git a/fs/orangefs/xattr.c b/fs/orangefs/xattr.c index 5893ddd..e4a070f 100644 --- a/fs/orangefs/xattr.c +++ b/fs/orangefs/xattr.c @@ -79,8 +79,8 @@ ssize_t orangefs_inode_getxattr(struct inode *inode, const char *prefix, return -EINVAL; } - fsuid = from_kuid(current_user_ns(), current_fsuid()); - fsgid = from_kgid(current_user_ns(), current_fsgid()); + fsuid = from_kuid(&init_user_ns, current_fsuid()); + fsgid = from_kgid(&init_user_ns, current_fsgid()); gossip_debug(GOSSIP_XATTR_DEBUG, "getxattr on inode %pU, name %s "