Re: [PATCH -mm 5/7] add user namespace

[Kirill Korotaev <dev@sw.ru>]

> > 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 :)
it doesn't matter :)
2.6.17 code is:
                       do_each_thread(g, p)
                               if (p->uid == who)
                                       error = set_one_prio(p, niceval, error);
                       while_each_thread(g, p);

when introducing process namespaces we will have to isolate processes somehow and this loop, agree?
in this case 1 user-namespace can belong to 2 process-namespaces, agree?
how do you see this loop in the future making sure that above situation is handled correctly?
how many other such places do we have?

> > 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.
the problem is not in find_user() actually. but in uid comparison inside
some kind of process iteration loop.
In this case you select processes p which belong to both namespaces simultenously:
i.e. processes p which belong both to user-namespace U and process-namespace P.

I hope I was more clear this time :)

> > 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().
why? we do it fine in OpenVZ.

> 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 :)

Kirill

-
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/