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

From: Kirill Korotaev
Date: Wed Jul 12 2006 - 07:20:21 EST


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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/