All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <christian.brauner@ubuntu.com>
To: cminyard@mvista.com
Cc: 0x7f454c46@gmail.com, areber@redhat.com, avagin@gmail.com,
	christian.brauner@ubuntu.com, linux-kernel@vger.kernel.org,
	minyard@acm.org, oleg@redhat.com
Subject: [PATCH] pid: make ENOMEM return value more obvious
Date: Sun,  8 Mar 2020 18:10:07 +0100	[thread overview]
Message-ID: <20200308171007.3155138-1-christian.brauner@ubuntu.com> (raw)
In-Reply-To: <20200307131136.GD2847@minyard.net>

The alloc_pid() codepath used to be simpler. With the introducation of the
ability to choose specific pids in 49cb2fc42ce4 ("fork: extend clone3() to
support setting a PID") it got more complex. It hasn't been super obvious
that ENOMEM is returned when the pid namespace init process/child subreaper
of the pid namespace has died. As can be seen from multiple attempts to
improve this see e.g. [1] and most recently [2].
We regressed returning ENOMEM in [3] and [2] restored it. Let's add a
comment on top explaining that this is historic and documented behavior and
cannot easily be changed.
The unconditional initialization of retval when declaring it can be removed
since it is initialized on ever failure path in the loop and unconditionaly
set to ENOMEM right after it.

[1]: 35f71bc0a09a ("fork: report pid reservation failure properly")
[2]: b26ebfe12f34 ("pid: Fix error return value in some cases")
[3]: 49cb2fc42ce4 ("fork: extend clone3() to support setting a PID")
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
---
 kernel/pid.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/pid.c b/kernel/pid.c
index 19645b25b77c..be43122eb876 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -165,7 +165,7 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
 	int i, nr;
 	struct pid_namespace *tmp;
 	struct upid *upid;
-	int retval = -ENOMEM;
+	int retval;
 
 	/*
 	 * set_tid_size contains the size of the set_tid array. Starting at
@@ -247,6 +247,14 @@ struct pid *alloc_pid(struct pid_namespace *ns, pid_t *set_tid,
 		tmp = tmp->parent;
 	}
 
+	/*
+	 * ENOMEM is not the most obvious choice especially for the case
+	 * where the child subreaper has already exited and the pid
+	 * namespace denies the creation of any new processes. But ENOMEM
+	 * is what we have exposed to userspace for a long time and it is
+	 * documented behavior for pid namespaces. So we can't easily
+	 * change it even if there were an error code better suited.
+	 */
 	retval = -ENOMEM;
 
 	if (unlikely(is_child_reaper(pid))) {

base-commit: b26ebfe12f34f372cf041c6f801fa49c3fb382c5
-- 
2.25.1


  parent reply	other threads:[~2020-03-08 17:10 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-06 17:23 [PATCH v2] pid: Fix error return value in some cases minyard
2020-03-07 11:00 ` Christian Brauner
2020-03-07 13:11   ` Corey Minyard
2020-03-08 17:07     ` Christian Brauner
2020-03-08 17:10     ` Christian Brauner [this message]
2020-03-08 17:16       ` [PATCH] selftests: add pid namespace ENOMEM regression test Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200308171007.3155138-1-christian.brauner@ubuntu.com \
    --to=christian.brauner@ubuntu.com \
    --cc=0x7f454c46@gmail.com \
    --cc=areber@redhat.com \
    --cc=avagin@gmail.com \
    --cc=cminyard@mvista.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=oleg@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.