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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110921191238.GA16720@sergelap>
Date:	Wed, 21 Sep 2011 14:12:38 -0500
From:	"Serge E. Hallyn" <serge.hallyn@...onical.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	lkml <linux-kernel@...r.kernel.org>, richard@....at,
	Andrew Morton <akpm@...gle.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Tejun Heo <tj@...nel.org>, serge@...lyn.com,
	Greg KH <greg@...ah.com>
Subject: Re: [PATCH] user namespace: usb: make usb urbs user namespace aware
 (v2)

Quoting Oleg Nesterov (oleg@...hat.com):
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/