On Fri, Mar 06, 2020 at 09:20:01AM -0600, minyard@acm.org wrote:
From: Corey Minyard cminyard@mvista.com
Recent changes to alloc_pid() allow the pid number to be specified on the command line. If set_tid_size is set, then the code scanning the levels will hard-set retval to -EPERM, overriding it's previous -ENOMEM value.
After the code scanning the levels, there are error returns that do not set retval, assuming it is still set to -ENOMEM.
In the first place, pid_ns_prepare_proc() returns its own error, just use that.
In the second place:
if (!(ns->pid_allocated & PIDNS_ADDING)) goto out_unlock;
a return value of -ENOMEM is probably wrong, since that means that the namespace is in deletion while this happened. -EINVAL is probably a better choice.
Fixes: 49cb2fc42ce4 "fork: extend clone3() to support setting a PID" Signed-off-by: Corey Minyard cminyard@mvista.com Cc: stable@vger.kernel.org # 5.5 Cc: Adrian Reber areber@redhat.com Cc: Christian Brauner christian.brauner@ubuntu.com Cc: Oleg Nesterov oleg@redhat.com Cc: Dmitry Safonov 0x7f454c46@gmail.com Cc: Andrei Vagin avagin@gmail.com Cc: Christian Brauner christian.brauner@ubuntu.com
kernel/pid.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/pid.c b/kernel/pid.c index 0f4ecb57214c..1921f7f4b236 100644 --- a/kernel/pid.c +++ b/kernel/pid.c @@ -248,7 +248,8 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid, } if (unlikely(is_child_reaper(pid))) {
if (pid_ns_prepare_proc(ns))
retval = pid_ns_prepare_proc(ns);if (retval) goto out_free;@@ -261,8 +262,10 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid, upid = pid->numbers + ns->level; spin_lock_irq(&pidmap_lock);
- if (!(ns->pid_allocated & PIDNS_ADDING))
- if (!(ns->pid_allocated & PIDNS_ADDING)) {
retval = -EINVAL;- } for ( ; upid >= pid->numbers; --upid) { /* Make the PID visible to find_pid_ns. */ idr_replace(&upid->ns->idr, pid, upid->nr);
Thanks for the patch.
We definitely regressed the ENOMEM return value case. But both of the changes here are userspace visible and break assumptions. So I'd rather just ensure that ENOMEM is returned in both cases like it was before. In fact, ENOMEM is documented on the pid_namespace man page so we can't really change it. Btw, this is the same problem as we had with:
commit 35f71bc0a09a45924bed268d8ccd0d3407bc476f Author: Michal Hocko mhocko@suse.cz Date: Thu Apr 16 12:47:38 2015 -0700
fork: report pid reservation failure properly
So please restore just restore the old behavior. Can you add --base to your next commit, please? Makes it easier for me to pick up.
Thanks! Christian