lkml.org 
[lkml]   [2006]   [Jul]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [PATCH -mm 5/7] add user namespace
    Kirill Korotaev wrote:
    > I wonder about another namespace coupling I found thinking
    > about your patches.
    >
    > Lets take a look at sys_setpriority() or any other function calling
    > find_user():
    > it can change the priority for all user or group processes like:
    >
    > do_each_thread_ve(g, p) {
    > if (p->uid == who)
    > error = set_one_prio(p, niceval, error);
    > } while_each_thread_ve(g, p);

    eh. this is openvz code ! thanks :)

    > which essentially means that user-namespace becomes coupled with
    > process-namespace. Sure, we can check in every such place for
    > p->nsproxy->user_ns == current->nsproxy->user_ns
    > condition. But this a way IMHO leading to kernel full of security
    > crap which is hardly maintainable.

    only 4 syscalls use find_user() : sys_setpriority, sys_getpriority,
    sys_ioprio_set, sys_ioprio_get and they use it very simply to check if a
    user_struct exists for a given uid. So, it should be OK. But please see the
    attached patch.

    > Another example of not so evident coupling here:
    > user structure maintains number of processes/opened
    > files/sigpending/locked_shm etc.
    > if a single user can belong to different proccess/ipc/... namespaces
    > all these becomes unusable.

    this is the purpose of execns.

    user namespace can't be unshared through the unshare syscall(). they can
    only be unshared through execns() which flushes the previous image of the
    process. However, the execns patch still needs to close files without the
    close-on-exec flag. I didn't do it yet. lazy me :)

    > Small patch comment:
    > - what is the reason in adding 2nd arg to find_user()?

    The main reason is alloc_uid() in clone_user_ns() and find_user() inherited
    the same prototype maybe abusively. Here's a patch.

    thanks,

    C.

    From: Cedric Le Goater <clg@fr.ibm.com>
    Subject: remove user namespace arg from find_user

    This patch removes the user namespace argument from find_user().

    find_user() is always called from the current context, hence the user
    namespace can be determined with current->nsproxy->user_ns in
    find_user().

    Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
    Cc: Andrew Morton <akpm@osdl.org>
    Cc: Kirill Korotaev <dev@openvz.org>
    Cc: Andrey Savochkin <saw@sw.ru>
    Cc: Eric W. Biederman <ebiederm@xmission.com>
    Cc: Herbert Poetzl <herbert@13thfloor.at>
    Cc: Sam Vilain <sam.vilain@catalyst.net.nz>
    Cc: Serge E. Hallyn <serue@us.ibm.com>
    Cc: Dave Hansen <haveblue@us.ibm.com>

    ---
    fs/ioprio.c | 4 ++--
    include/linux/sched.h | 2 +-
    kernel/sys.c | 4 ++--
    kernel/user.c | 3 ++-
    4 files changed, 7 insertions(+), 6 deletions(-)

    Index: 2.6.18-rc1-mm1/fs/ioprio.c
    ===================================================================
    --- 2.6.18-rc1-mm1.orig/fs/ioprio.c
    +++ 2.6.18-rc1-mm1/fs/ioprio.c
    @@ -102,7 +102,7 @@ asmlinkage long sys_ioprio_set(int which
    if (!who)
    user = current->user;
    else
    - user = find_user(current->nsproxy->user_ns, who);
    + user = find_user(who);

    if (!user)
    break;
    @@ -172,7 +172,7 @@ asmlinkage long sys_ioprio_get(int which
    if (!who)
    user = current->user;
    else
    - user = find_user(current->nsproxy->user_ns, who);
    + user = find_user(who);

    if (!user)
    break;
    Index: 2.6.18-rc1-mm1/include/linux/sched.h
    ===================================================================
    --- 2.6.18-rc1-mm1.orig/include/linux/sched.h
    +++ 2.6.18-rc1-mm1/include/linux/sched.h
    @@ -539,7 +539,7 @@ struct user_struct {
    uid_t uid;
    };

    -extern struct user_struct *find_user(struct user_namespace *, uid_t);
    +extern struct user_struct *find_user(uid_t);

    extern struct user_struct root_user;
    #define INIT_USER (&root_user)
    Index: 2.6.18-rc1-mm1/kernel/sys.c
    ===================================================================
    --- 2.6.18-rc1-mm1.orig/kernel/sys.c
    +++ 2.6.18-rc1-mm1/kernel/sys.c
    @@ -545,7 +545,7 @@ asmlinkage long sys_setpriority(int whic
    who = current->uid;
    else
    if ((who != current->uid) &&
    - !(user = find_user(current->nsproxy->user_ns, who)))
    + !(user = find_user(who)))
    goto out_unlock; /* No processes for this user */

    do_each_thread(g, p)
    @@ -604,7 +604,7 @@ asmlinkage long sys_getpriority(int whic
    who = current->uid;
    else
    if ((who != current->uid) &&
    - !(user = find_user(current->nsproxy->user_ns, who)))
    + !(user = find_user(who)))
    goto out_unlock; /* No processes for this user */

    do_each_thread(g, p)
    Index: 2.6.18-rc1-mm1/kernel/user.c
    ===================================================================
    --- 2.6.18-rc1-mm1.orig/kernel/user.c
    +++ 2.6.18-rc1-mm1/kernel/user.c
    @@ -205,10 +205,11 @@ void free_user_ns(struct kref *kref)
    *
    * If the user_struct could not be found, return NULL.
    */
    -struct user_struct *find_user(struct user_namespace *ns, uid_t uid)
    +struct user_struct *find_user(uid_t uid)
    {
    struct user_struct *ret;
    unsigned long flags;
    + struct user_namespace *ns = current->nsproxy->user_ns;

    spin_lock_irqsave(&uidhash_lock, flags);
    ret = uid_hash_find(uid, uidhashentry(ns, uid));
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-07-11 19:41    [W:3.366 / U:0.392 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site