Messages in this thread | | | Date | Thu, 27 Apr 2017 18:12:57 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/2] pid_ns: Introduce ioctl to set vector of ns_last_pid's on ns hierarhy |
| |
On 04/26, Kirill Tkhai wrote: > > On 26.04.2017 18:53, Oleg Nesterov wrote: > > > >> +static long set_last_pid_vec(struct pid_namespace *pid_ns, > >> + struct pidns_ioc_req *req) > >> +{ > >> + char *str, *p; > >> + int ret = 0; > >> + pid_t pid; > >> + > >> + read_lock(&tasklist_lock); > >> + if (!pid_ns->child_reaper) > >> + ret = -EINVAL; > >> + read_unlock(&tasklist_lock); > >> + if (ret) > >> + return ret; > > > > why do you need to check ->child_reaper under tasklist_lock? this looks pointless. > > > > In fact I do not understand how it is possible to hit pid_ns->child_reaper == NULL, > > there must be at least one task in this namespace, otherwise you can't open a file > > which has f_op == ns_file_operations, no? > > Sure, it's impossible to pick a pid_ns, if there is no the pid_ns's tasks. I added > it under impression of > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=dfda351c729733a401981e8738ce497eaffcaa00 > but here it's completely wrong. It will be removed in v2.
Hmm. But if I read this commit correctly then we really need to check pid_ns->child_reaper != NULL ?
Currently we can't pick an "empty" pid_ns. But after the commit above a task can do sys_unshare(CLONE_NEWPID), another (or the same) task can open its /proc/$pid/ns/pid_for_children and call ns_ioctl() before the 1st alloc_pid() ?
Or I am totally confused?
Oleg.
| |