"Serge E. Hallyn" <serge@xxxxxxxxxx> writes:
> Quoting Eric W. Beiderman (ebiederm@xxxxxxxxxxxx):
>> From: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
>>
>> - Transform userns->creator from a user_struct reference to a simple
>> kuid_t, kgid_t pair.
>>
>> In cap_capable this allows the check to see if we are the creator of
>> a namespace to become the classic suser style euid permission check.
>>
>> This allows us to remove the need for a struct cred in the mapping
>> functions and still be able to dispaly the user namespace creators
>> uid and gid as 0.
>>
>> - Remove the now unnecessary delayed_work in free_user_ns.
>>
>> All that is left for free_user_ns to do is to call kmem_cache_free
>> and put_user_ns. Those functions can be called in any context
>> so call them directly from free_user_ns removing the need for delayed work.
>>
>> Signed-off-by: Eric W. Biederman <ebiederm@xxxxxxxxxxxx>
>> ---
>> include/linux/user_namespace.h | 4 ++--
>> kernel/user.c | 7 ++++---
>> kernel/user_namespace.c | 39 ++++++++++++++++++---------------------
>> security/commoncap.c | 5 +++--
>> 4 files changed, 27 insertions(+), 28 deletions(-)
>>
>> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
>> index d767508..8a391bd 100644
>> --- a/include/linux/user_namespace.h
>> +++ b/include/linux/user_namespace.h
>> @@ -9,8 +9,8 @@
>> struct user_namespace {
>> struct kref kref;
>> struct user_namespace *parent;
>> - struct user_struct *creator;
>> - struct work_struct destroyer;
>> + kuid_t owner;
>> + kgid_t group;
>> };
>>
>> extern struct user_namespace init_user_ns;
>> diff --git a/kernel/user.c b/kernel/user.c
>> index 025077e..cff3856 100644
>> --- a/kernel/user.c
>> +++ b/kernel/user.c
>> @@ -25,7 +25,8 @@ struct user_namespace init_user_ns = {
>> .kref = {
>> .refcount = ATOMIC_INIT(3),
>> },
>> - .creator = &root_user,
>> + .owner = GLOBAL_ROOT_UID,
>> + .group = GLOBAL_ROOT_GID,
>> };
>> EXPORT_SYMBOL_GPL(init_user_ns);
>>
>> @@ -54,9 +55,9 @@ struct hlist_head uidhash_table[UIDHASH_SZ];
>> */
>> static DEFINE_SPINLOCK(uidhash_lock);
>>
>> -/* root_user.__count is 2, 1 for init task cred, 1 for init_user_ns->user_ns */
>> +/* root_user.__count is 1, for init task cred */
>> struct user_struct root_user = {
>> - .__count = ATOMIC_INIT(2),
>> + .__count = ATOMIC_INIT(1),
>> .processes = ATOMIC_INIT(1),
>> .files = ATOMIC_INIT(0),
>> .sigpending = ATOMIC_INIT(0),
>> diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c
>> index 898e973..f69741a 100644
>> --- a/kernel/user_namespace.c
>> +++ b/kernel/user_namespace.c
>> @@ -27,6 +27,16 @@ int create_user_ns(struct cred *new)
>> {
>> struct user_namespace *ns, *parent_ns = new->user_ns;
>> struct user_struct *root_user;
>> + kuid_t owner = make_kuid(new->user_ns, new->euid);
>> + kgid_t group = make_kgid(new->user_ns, new->egid);
>> +
>> + /* The creator needs a mapping in the parent user namespace
>> + * or else we won't be able to reasonably tell userspace who
>> + * created a user_namespace.
>> + */
>> + if (!kuid_has_mapping(parent_ns, owner) ||
>> + !kgid_has_mapping(parent_ns, group))
>> + return -EPERM;
>>
>> ns = kmem_cache_alloc(user_ns_cachep, GFP_KERNEL);
>> if (!ns)
>> @@ -43,7 +53,9 @@ int create_user_ns(struct cred *new)
>>
>> /* set the new root user in the credentials under preparation */
>> ns->parent = parent_ns;
>
> I think in the past the creator cred pinned the ns->parent. Do you now
> need to explicitly pin ns->parent (and release it in free_user_ns())?
Yes we do have to explicitly reference count the parent namespace.
But that happened in the patch 7:
"userns: Add an explicit reference to the parent user namespace"
Eric
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/containers
