linux-kernel - Re: [PATCH v3 1/3] pidfd: allow pidfd_open() on non-thread-group leaders

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20240127-welpen-seitlich-cd8707e09d1a@brauner>
Date: Sat, 27 Jan 2024 15:26:37 +0100
From: Christian Brauner <brauner@...nel.org>
To: Oleg Nesterov <oleg@...hat.com>
Cc: Tycho Andersen <tycho@...ho.pizza>, linux-kernel@...r.kernel.org, 
	linux-api@...r.kernel.org, Tycho Andersen <tandersen@...flix.com>, 
	"Eric W. Biederman" <ebiederm@...ssion.com>
Subject: Re: [PATCH v3 1/3] pidfd: allow pidfd_open() on non-thread-group
 leaders

On Fri, Jan 26, 2024 at 03:33:50PM +0100, Oleg Nesterov wrote:
> On 01/26, Christian Brauner wrote:
> >
> > > --- a/include/uapi/linux/pidfd.h
> > > +++ b/include/uapi/linux/pidfd.h
> > > @@ -7,6 +7,7 @@
> > >  #include <linux/fcntl.h>
> > >
> > >  /* Flags for pidfd_open().  */
> > > -#define PIDFD_NONBLOCK O_NONBLOCK
> > > +#define PIDFD_NONBLOCK	O_NONBLOCK
> > > +#define PIDFD_THREAD	O_EXCL	// or anything else not used by anon_inode's
> >
> > I like it!
> >
> > The only request I would have is to not alias O_EXCL and PIDFD_THREAD.
> > Because it doesn't map as clearly as NONBLOCK did.
> 
> But it would be nice to have PIDFD_THREAD in file->f_flags. Where else
> can we keep it?

No, I did just mean that the uapi value doesn't necessarily have to
reflect what we do internally. IOW, we can still raise O_EXCL internally
in ->f_flags but there's no need to expose it as O_EXCL to userspace. We
often have internal and external flag spaces. If you prefer it your way
I'm not going argue.

> I chose O_EXCL because it can only be used at open time, it can never
> be used or changed after anon_inode_getfile(), so we can safely do
> 
> 	pidfd_file->f_flags |= PIDFD_THREAD;
> 
> in __pidfd_prepare() and then check in pidfd_poll/pidfd_send_signal.
> 
> What do you suggest instead?

(Long-term and unrelated to this here, I think we will need to consider
not just stashing struct pid in pidfd_file->private_data but instead
struct pid with additional data because there's various functionality
that users would like that requires additional state to be stored and we
can't or don't want to do that in struct pid directly.)