Re: proc_flush_task oops

[Alexey Dobriyan]

On 12/21/17, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote:
> I have stared at this code, and written some test programs and I can't
> see what is going on.  alloc_pid by design and in implementation (as far
> as I can see) is always single threaded when allocating the first pid
> in a pid namespace.  idr_init always initialized idr_next to 0.
>
> So how we can get past:
>
> 	if (unlikely(is_child_reaper(pid))) {
> 		if (pid_ns_prepare_proc(ns)) {
> 			disable_pid_allocation(ns);
> 			goto out_free;
> 		}
> 	}
>
> with proc_mnt still set to NULL is a mystery to me.
>
> Is there any chance the idr code doesn't always return the lowest valid
> free number?  So init gets assigned something other than 1?

Well, this theory is easy to test (attached).

There is a "valid" way to break the code via kernel.ns_last_pid:
unshare+write+fork but the reproducer doesn't seem to use it (or it does?)
diff --git a/include/linux/pid_namespace.h b/include/linux/pid_namespace.h
index 49538b172483..9829e74c9bd7 100644
--- a/include/linux/pid_namespace.h
+++ b/include/linux/pid_namespace.h
@@ -44,6 +44,7 @@ struct pid_namespace {
 	kgid_t pid_gid;
 	int hide_pid;
 	int reboot;	/* group exit code if this pidns was rebooted */
+	bool pid1;
 	struct ns_common ns;
 } __randomize_layout;
 
diff --git a/kernel/pid.c b/kernel/pid.c
index a7f38aff740a..c2af4d420e2a 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -199,6 +199,15 @@ struct pid *alloc_pid(struct pid_namespace *ns)
 			goto out_free;
 		}
 
+
+		if (ns->pid1) {
+			if (nr != 1) {
+				printk("%s: pid %d\n", __func__, nr);
+				BUG();
+			}
+			ns->pid1 = false;
+		}
+
 		pid->numbers[i].nr = nr;
 		pid->numbers[i].ns = tmp;
 		tmp = tmp->parent;
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 0b53eef7d34b..234bf7c2d53f 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -135,6 +135,7 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns
 	ns->ucounts = ucounts;
 	ns->pid_allocated = PIDNS_ADDING;
 	INIT_WORK(&ns->proc_work, proc_cleanup_work);
+	ns->pid1 = true;
 
 	return ns;