From: ebiederm@xmission.com (Eric W. Biederman)
To: Dave Jones <davej@codemonkey.org.uk>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Linux Kernel <linux-kernel@vger.kernel.org>,
        syzkaller-bugs@googlegroups.com, Gargi Sharma <gs051095@gmail.com>,
        Oleg Nesterov <oleg@redhat.com>, Rik van Riel <riel@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>
References: <20171219033926.GA26981@codemonkey.org.uk>
        <87lghy7eul.fsf@xmission.com>
        <20171219193020.GA9237@codemonkey.org.uk>
        <878tdy5r5t.fsf@xmission.com> <87mv2e17vz.fsf@xmission.com>
        <20171220052803.GA17079@codemonkey.org.uk>
        <871sjp1cjz.fsf@xmission.com>
        <20171221031606.GA4636@codemonkey.org.uk>
        <87po78trjm.fsf@xmission.com>
        <CACVxJT_Sud7tNv23yOOqG7Uh5zFPDnz1anS7pTr0ChpCSMvZDA@mail.gmail.com>
        <20171221220044.GA4977@codemonkey.org.uk>
Date: Thu, 21 Dec 2017 19:31:26 -0600
In-Reply-To: <20171221220044.GA4977@codemonkey.org.uk> (Dave Jones's message
        of "Thu, 21 Dec 2017 17:00:44 -0500")
Message-ID: <87wp1fk0pd.fsf@xmission.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: proc_flush_task oops
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2079
Lines: 59

Dave Jones <davej@codemonkey.org.uk> writes:

> On Thu, Dec 21, 2017 at 12:38:12PM +0200, Alexey Dobriyan wrote:
>  
>  > > 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).
>
> I didn't hit this BUG, but I hit the same oops in proc_flush_task.

Scratch one idea.

If it isn't too much trouble can you try this.

I am wondering if somehow the proc_mnt that is NULL is somewhere in the
middle of the stack of pid namespaces.

This adds two warnings.  The first just reports which pid namespace in
the stack of pid namespaces is problematic, and the pid number in that
pid namespace.  Which should give a whole lot more to go by.

The second warning complains if we manage to create a pid namespace
where the parent pid namespace is not properly set up.  The test to
prevent that looks quite robust, but at this point I don't know where to
look.

Thank you very much,
Eric


diff --git a/kernel/pid.c b/kernel/pid.c
index b13b624e2c49..a1e8734afbba 100644
--- a/kernel/pid.c
+++ b/kernel/pid.c
@@ -210,6 +210,7 @@ struct pid *alloc_pid(struct pid_namespace *ns)
                goto out_unlock;
        for ( ; upid >= pid->numbers; --upid) {
                /* Make the PID visible to find_pid_ns. */
+               WARN(!upid->ns->proc_mnt, "%ld/%d: %d no proc_mnt", (upid - pid->numbers), pid->level, upid->nr);
                idr_replace(&upid->ns->idr, pid, upid->nr);
                upid->ns->pid_allocated++;
        }
diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 0b53eef7d34b..8f4c02c7223a 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -136,6 +136,8 @@ static struct pid_namespace *create_pid_namespace(struct user_namespace *user_ns
        ns->pid_allocated = PIDNS_ADDING;
        INIT_WORK(&ns->proc_work, proc_cleanup_work);
 
+       WARN_ON(!parent_pid_ns->proc_mnt);
+
        return ns;
 
 out_free_idr: