linux-kernel - Re: [PATCH] user namespace: make signal.c respect user namespaces

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: <20110920164524.GA8214@sergelap>
Date:	Tue, 20 Sep 2011 11:45:24 -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
Subject: Re: [PATCH] user namespace: make signal.c respect user namespaces

Quoting Oleg Nesterov (oleg@...hat.com):
> On 09/20, Serge Hallyn wrote:
> >
> > Quoting Oleg Nesterov (oleg@...hat.com):
> > >
> > > As for __send_signal() in particular, I guess we could do
> > >
> > > 		q->info.si_uid = from_ancestor_ns ? 0 : current_uid();
> >
> > But wait - can't __send_signal end up being triggered through fcntl
> > F_SETSIG+F_SETOWN to another but none-ancestor namespace?
> 
> I didn't really check, but afaics send_sigio_to_task/etc never use
> SEND_SIG_NOINFO. And they shouldn't, there is no the sending process.

(I'm sure you're right - but I'll have to take a closer look for my
own understanding)

> > do actually need to call the full user_ns_map_uid() there.  However,
> > the uid sent in ptrace_signal() can (if i'm thinking right) be set
> > using
> >
> > 	info->si_uid = (current_user_ns() == parent_user_ns()) ?
> > 			0 : current_uid();
> >
> > Not much of a win performance-wise, but perhaps better by making it
> > clear that those are the only possibilities (and overflowuid is not
> > possible).
> 
> I think you are right right, except s/==/!=/.

Yes :)

> This is like send_signal()
> which can only send the signal down or to the same namespace.
> 
> I guess ptrace_signal() can even do
> 
> 	info->si_pid = task_pid_vnr(current->parent);
> 
> 	if (info->si_pid)	// same namespace
> 		info->si_uid = current_uid();
> 	else			// can't see the tracer, we are from the sub-namespace
> 		info->si_uid = 0;
> 
> Whatever you like more.

Except you don't have to split pid and user namespaces at the same
time.

> And just in case let me repeat, I won't argue if you use
> user_ns_map_uid/helper for consistency. Without CONFIG_USER_NS it is
> even better performance-wise (even if we add a helper I guess).

Ok.  On the one hand, using your new mapping helper will make it
easier to make sure everything is right.  This special code may be
more educational to the new reader, but isn't as trivial to
review.

Maybe I'll go with the mapping.

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/