Re: [PATCH] user namespace: usb: make usb urbs user namespace aware(v2)

From: Serge E. Hallyn
Date: Wed Sep 21 2011 - 15:12:47 EST


Quoting Oleg Nesterov (oleg@xxxxxxxxxx):
> On 09/21, Serge E. Hallyn wrote:
> >
> > Add to the dev_state and alloc_async structures the user namespace
> > corresponding to the uid and euid. Pass these to kill_pid_info_as_uid(),
> > which can then implement a proper, user-namespace-aware uid check.
>
> Looks correct.
>
>
>
> But I have off-topic question. And in fact I am a bit confused,
> please help.
>
> First of all, I assume that CLONE_NEWUSER is the only way to change
> ->user_ns, right?

Yes.

> And, looking at copy_creds() I think that cred->user_ns is always
> equal to cred->user->user_ns. However, grep shows a lot of
> cred->user->user_ns examples. Why?

Good question. It's only because cred->user_ns is an optimization
recently introduced. I think those can be safely switched over.

> > +static int kill_as_cred_perm(const struct cred *cred,
> > + struct task_struct *target)
> > +{
> > + const struct cred *pcred = __task_cred(target);
> > + if (cred->user_ns != pcred->user_ns)
> > + return 0;
>
> Should we really fail if cred->user_ns == pcred->user_ns->creator ?
> (or creator of creator, etc).
>
> IOW, shouldn't this match kill_ok_by_cred() path which (at least
> cap_capable) checks the ->creator chain when ->user_ns differ?

I'm not sure. We can relax that later if need be, but as this has to do
with usb urbs and userspace drivers, I don't think we'll want to.
Hopefully we can talk with Greg KH about it at some point. But for now,
with this patch, all interactions between tasks in the initial user
namespace will continue as normal, and we're not allowing anything
untoward between user namespaces, so I think this is best.

Drat. Greg, sorry about not Cc:ing you on the original patch. Please
let me know if you'd like me to resend to you.

thanks,
-serge
--
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/