linux-kernel - Re: [PATCH v7 2/2] selftests: add tests for pidfd_send

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190108182444.e6kq2gqftde23jou@brauner.io>
Date:   Tue, 8 Jan 2019 19:24:46 +0100
From:   Christian Brauner <christian@...uner.io>
To:     Tycho Andersen <tycho@...ho.ws>
Cc:     "Serge E. Hallyn" <serge@...lyn.com>, linux-kernel@...r.kernel.org,
        linux-api@...r.kernel.org, luto@...nel.org, arnd@...db.de,
        keescook@...omium.org, akpm@...ux-foundation.org, jannh@...gle.com,
        oleg@...hat.com, cyphar@...har.com, viro@...iv.linux.org.uk,
        linux-fsdevel@...r.kernel.org, dancol@...gle.com,
        timmurray@...gle.com, fweimer@...hat.com, tglx@...utronix.de,
        x86@...nel.org, ebiederm@...ssion.com
Subject: Re: [PATCH v7 2/2] selftests: add tests for pidfd_send_signal()

On Tue, Jan 08, 2019 at 11:20:23AM -0700, Tycho Andersen wrote:
> On Tue, Jan 08, 2019 at 12:17:42PM -0600, Serge E. Hallyn wrote:
> > On Tue, Jan 08, 2019 at 10:58:43AM -0700, Tycho Andersen wrote:
> > > On Tue, Jan 08, 2019 at 11:54:15AM -0600, Serge E. Hallyn wrote:
> > > > On Tue, Jan 08, 2019 at 10:53:06AM -0700, Tycho Andersen wrote:
> > > > > On Wed, Jan 02, 2019 at 05:16:54PM +0100, Christian Brauner wrote:
> > > > > > +			/*
> > > > > > +			 * Stop the child so we can inspect whether we have
> > > > > > +			 * recycled pid PID_RECYCLE.
> > > > > > +			 */
> > > > > > +			close(pipe_fds[0]);
> > > > > > +			ret = kill(recycled_pid, SIGSTOP);
> > > > > > +			close(pipe_fds[1]);
> > > > > > +			if (ret) {
> > > > > > +				(void)wait_for_pid(recycled_pid);
> > > > > > +				_exit(PIDFD_ERROR);
> > > > > > +			}
> > > > > 
> > > > > Sorry for being late to the party, but I wonder if this whole thing
> > > > > couldn't be simplified with /proc/sys/kenrel/ns_last_pid?
> > > > 
> > > > no, bc it's not namespaced :)
> > > 
> > > Huh? It looks like it is...
> > > 
> > > static int pid_ns_ctl_handler(struct ctl_table *table, int write,
> > >                 void __user *buffer, size_t *lenp, loff_t *ppos)
> > > {
> > >         struct pid_namespace *pid_ns = task_active_pid_ns(current);
> > >         struct ctl_table tmp = *table;
> > >         int ret, next;
> > > 
> > >         if (write && !ns_capable(pid_ns->user_ns, CAP_SYS_ADMIN))
> > >                 return -EPERM;
> > > 
> > >         ...
> > 
> > Oh - hah, but that's ns_last_pid.  You'd want pid_max.  And that one
> > is not namespaced.
> 
> Perhaps I'm misunderstanding, but isn't the point of all this code to
> get the same pid again? So can't we just fork(), kill(), then set
> ns_last_pid to pid-1, and fork() again to re-use?

Maybe. It's just a selftest that works reliably as it is so unless
there's a technical issue with the patch I'm not going to do another
version just because of that unless people feel super strongly about
this.
Another advantage is that the code we have right now works even when
CONFIG_CHECKPOINT_RESTORE is not selected.