Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Wed, 20 Jun 2018 08:41:25 -0600
From:   Tycho Andersen <tycho@tycho.ws>
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     Matthew Helsley <matt.helsley@gmail.com>,
        Kees Cook <keescook@chromium.org>,
        lkml <linux-kernel@vger.kernel.org>,
        containers@lists.linux-foundation.org,
        Oleg Nesterov <oleg@redhat.com>,
        Akihiro Suda <suda.akihiro@lab.ntt.co.jp>,
        Tyler Hicks <tyhicks@canonical.com>,
        Christian Brauner <christian.brauner@ubuntu.com>,
        Andy Lutomirski <luto@amacapital.net>,
        "Tobin C . Harding" <me@tobin.cc>
Subject: Re: [PATCH v3 1/4] seccomp: add a return code to trap to userspace
Message-ID: <20180620144125.GH14770@smitten>
References: <20180531144949.24995-1-tycho@tycho.ws>
 <20180531144949.24995-2-tycho@tycho.ws>
 <CA+RrjuW98m2coL+TOKq5cL0QhAb=HYxo2DpNoqMzdiwjqhc2BA@mail.gmail.com>
 <20180612231610.GA3837@cisco>
 <CA+RrjuUtYoXfbH3cTbSY=QzXcxJsJOa0BL628ADy9N3bTO4=Mw@mail.gmail.com>
 <20180614210325.GA5673@cisco>
 <87in6lt4pc.fsf@xmission.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87in6lt4pc.fsf@xmission.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Eric,

On Thu, Jun 14, 2018 at 04:53:51PM -0500, Eric W. Biederman wrote:
> >> static void seccomp_do_user_notification(...)
> >> {
> >>     ...
> >>     n.pid = get_task_pid(current, PIDTYPE_PID);
> >>     ...
> >> remove_list:
> >>     list_del(&n.list);
> >>     put_pid(n.pid);
> >>     ...
> >> }
> >> ...
> >> static ssize_t seccomp_notify_read(...)
> >> {
> >>     ...
> >>     unotif.pid = pid_vnr(knotif->pid);
> >>     ...
> >> }
> >> 
> >> I like holding the pid reference because it's what we do elsewhere when pid
> >> namespaces
> >> are a concern and it more precisely specifies what the knotif content needs
> >> to convey.
> >> Otherwise I don't think it makes a difference.
> >
> > Great, thanks, I'll do this. I guess we need a put_pid() here too.
> 
> A)  We know that the task is stopped.  Unless there is something
>     like SIGKILL that can make the task move you don't need to
>     take a reference to anything.

Yes, agreed. (I think the task can't die, because even if it gets an
interrupt, we hold the ->notify_lock here, so it'll block waiting for
that to remove itself from the notification queue.)

> B)  pid_vnr is the wrong answer.  When you create the struct file
>     and intialize the filter you need to capture the calling processes
>     pid namespace.  The you can use "pid_nr_ns(knotif->pid, filter->pid_ns);".
>     That will work consistently even if the file descriptor is passed
>     between processes.

We want the pid of the tracee in the tracer's namespace, so I'm not so
sure. Doesn't your code above give us the pid in the namespace of the
task that happened to create the struct file (which may be unrelated
to the namespace of the tracer)?

Tycho