linux-kernel - Re: [RFC][PATCH 0/3] fork: Add the ability to create tasks with given pids

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: <20111122211645.GA21608@redhat.com>
Date:	Tue, 22 Nov 2011 22:16:45 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Tejun Heo <tj@...nel.org>
Cc:	Pavel Emelyanov <xemul@...allels.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Alan Cox <alan@...ux.intel.com>,
	Roland McGrath <roland@...k.frob.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Cyrill Gorcunov <gorcunov@...nvz.org>,
	James Bottomley <jbottomley@...allels.com>
Subject: Re: [RFC][PATCH 0/3] fork: Add the ability to create tasks with
	given pids

On 11/21, Tejun Heo wrote:
>
> > +static int pid_ns_ctl_handler(struct ctl_table *table, int write,
> > +		     void __user *buffer, size_t *lenp, loff_t *ppos)
> > +{
> > +	struct ctl_table tmp = *table;
> > +	tmp.data = &current->nsproxy->pid_ns->last_pid;
> > +	return proc_dointvec(&tmp, write, buffer, lenp, ppos);
> > +}
>
> Probably better to call set_last_pid() on write path instead?

I am not sure... set_last_pid() is "special", it tries to avoid the
races with itself to prevent the pid-reuse. It doesn't hurt, but
perhaps

	set_last_pid(pid_ns, pid_ns->last_pid, new_pid);

looks a bit confusing.

Hmm. On the second thought, perhaps this makes sense anyway. Just
to keep the "only set_last_pid() can change ->last_pid" property.

But this is almost cosmetic.

> > Well, after a bit more thinking I found one more pros for this
> > sysctl - when restoring a container we'll have the possibility to
> > set the last_pid to what we want to prevent the pids reuse after the
> > restore.
>
> Hmmm... I personally like this one better.  Restoring multilevel pid
> would be more tedious but should still be possible and I really like
> that it's staying out of clone path and all modifications are to ns
> and pid code.  Oleg, what do you think?

Obviously, I'd prefer this one too ;)

But. Personally I do not like the fact that only init can open this
file for writing... (I guess Pavel already hates me ;)

If we add this sysctl, then I think there should be some way to use
outside of "checkpoint-restore" world. For example, see the comment
from Pedro. This use-case looks unexpected (to me), but reasonable.
Or. Say, set_last_pid can be useful to test the pid-reuse races.

In any case. To me, it is not really good to have /proc/*/set_last_pid
without the ability to use it somehow on the running system.

Oleg.

--
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/