From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932882Ab1JXPrF (ORCPT ); Mon, 24 Oct 2011 11:47:05 -0400 Received: from mail-yx0-f174.google.com ([209.85.213.174]:42708 "EHLO mail-yx0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932793Ab1JXPrD convert rfc822-to-8bit (ORCPT ); Mon, 24 Oct 2011 11:47:03 -0400 MIME-Version: 1.0 In-Reply-To: <20111024144334.GA26603@hallyn.com> References: <1318974898-21431-1-git-send-email-serge@hallyn.com> <1318974898-21431-6-git-send-email-serge@hallyn.com> <14652.1319014868@redhat.com> <20111024144334.GA26603@hallyn.com> Date: Mon, 24 Oct 2011 08:47:01 -0700 X-Google-Sender-Auth: MVyHSxbhtMgc3UBw5lMwUU1VGug Message-ID: Subject: Re: [PATCH 05/10] user namespace: clamp down users of cap_raised From: "Andrew G. Morgan" To: "Serge E. Hallyn" Cc: David Howells , linux-kernel@vger.kernel.org, ebiederm@xmission.com, akpm@linux-foundation.org, oleg@redhat.com, richard@nod.at, mikevs@xs4all.net, segoon@openwall.com, gregkh@suse.de, eparis@redhat.com, "Serge E. Hallyn" Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Serge, It seems as if this whole thing is really idiomatic. How about? #define IN_ROOT_USER_NS_CAPABLE(cap) \ ((current_user_ns() == &init_user_ns) && cap_raised(current_cap(), cap)) Cheers Andrew On Mon, Oct 24, 2011 at 7:43 AM, Serge E. Hallyn wrote: > A few modules are using cap_raised(current_cap(), cap) to authorize > actions.  This means that tasks which are privileged in non-initial > user namespaces will be deemed privileged.  The privilege should only > be granted if the task is in the initial user namespace. > > Switching the calls to capable() would change the behavior - it would > cause the LSM capable hooks to be called, and set PF_SUPERPRIV if > the capability was used.  So instead, put in an explicit check and > refuse privilege if the caller is not in init_user_ns. > > Changelog: > Oct 23: Use a nice macro to make the code shorter and easier to read, >        per advice from Andrew Morgan and David Howells. > > Signed-off-by: Serge E. Hallyn > Cc: Eric W. Biederman > Cc: Andrew Morgan > Cc: Vasiliy Kulikov > Cc: David Howells > --- >  drivers/block/drbd/drbd_nl.c           |    2 +- >  drivers/md/dm-log-userspace-transfer.c |    2 +- >  drivers/staging/pohmelfs/config.c      |    2 +- >  drivers/video/uvesafb.c                |    2 +- >  include/linux/cred.h                   |    2 ++ >  5 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c > index 0feab26..f799b15 100644 > --- a/drivers/block/drbd/drbd_nl.c > +++ b/drivers/block/drbd/drbd_nl.c > @@ -2297,7 +2297,7 @@ static void drbd_connector_callback(struct cn_msg *req, struct netlink_skb_parms >                return; >        } > > -       if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) { > +       if (!IN_ROOT_USER_NS() || !cap_raised(current_cap(), CAP_SYS_ADMIN)) { >                retcode = ERR_PERM; >                goto fail; >        } > diff --git a/drivers/md/dm-log-userspace-transfer.c b/drivers/md/dm-log-userspace-transfer.c > index 1f23e04..126a79b 100644 > --- a/drivers/md/dm-log-userspace-transfer.c > +++ b/drivers/md/dm-log-userspace-transfer.c > @@ -134,7 +134,7 @@ static void cn_ulog_callback(struct cn_msg *msg, struct netlink_skb_parms *nsp) >  { >        struct dm_ulog_request *tfr = (struct dm_ulog_request *)(msg + 1); > > -       if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) > +       if (!IN_ROOT_USER_NS() || !cap_raised(current_cap(), CAP_SYS_ADMIN)) >                return; > >        spin_lock(&receiving_list_lock); > diff --git a/drivers/staging/pohmelfs/config.c b/drivers/staging/pohmelfs/config.c > index b6c42cb..327c047 100644 > --- a/drivers/staging/pohmelfs/config.c > +++ b/drivers/staging/pohmelfs/config.c > @@ -525,7 +525,7 @@ static void pohmelfs_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *n >  { >        int err; > > -       if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) > +       if (!IN_ROOT_USER_NS() || !cap_raised(current_cap(), CAP_SYS_ADMIN)) >                return; > >        switch (msg->flags) { > diff --git a/drivers/video/uvesafb.c b/drivers/video/uvesafb.c > index 7f8472c..94e0e9d 100644 > --- a/drivers/video/uvesafb.c > +++ b/drivers/video/uvesafb.c > @@ -73,7 +73,7 @@ static void uvesafb_cn_callback(struct cn_msg *msg, struct netlink_skb_parms *ns >        struct uvesafb_task *utask; >        struct uvesafb_ktask *task; > > -       if (!cap_raised(current_cap(), CAP_SYS_ADMIN)) > +       if (!IN_ROOT_USER_NS() || !cap_raised(current_cap(), CAP_SYS_ADMIN)) >                return; > >        if (msg->seq >= UVESAFB_TASKS_MAX) > diff --git a/include/linux/cred.h b/include/linux/cred.h > index 4030896..2f75da7 100644 > --- a/include/linux/cred.h > +++ b/include/linux/cred.h > @@ -359,9 +359,11 @@ static inline void put_cred(const struct cred *_cred) > >  #ifdef CONFIG_USER_NS >  #define current_user_ns() (current_cred_xxx(user_ns)) > +#define IN_ROOT_USER_NS() (current_user_ns() == &init_user_ns) >  #else >  extern struct user_namespace init_user_ns; >  #define current_user_ns() (&init_user_ns) > +#define IN_ROOT_USER_NS() (1) >  #endif > > > -- > 1.7.5.4 > >