Re: [PATCH] [GLIBC RFC] clone3: add CLONE3_RESET_SIGHAND

[Christian Brauner <christian dot brauner at ubuntu dot com>]

On Wed, Oct 09, 2019 at 03:33:41PM +0200, Christian Brauner wrote:
> On Wed, Oct 09, 2019 at 02:01:02PM +0200, Florian Weimer wrote:
> > * Christian Brauner:
> > 
> > >> With this construct, the application programmer needs to remember which
> > >> flags are old and new (predate and postdate known_flags).  It's too easy
> > >> to make mistakes there.
> > >> 
> > >> What about this?
> > >> 
> > >>   pid_t pid = clone3 (&args, sizeof (args));
> > >>   if (pid < 0)
> > >>     return -1;
> > >> 
> > >>   if (args.known_flags == 0)
> > >>     args.known_flags = CLONE3_INITIALLY_SUPPORTED_FLAGS;
> > >> 
> > >>   if (args.known_flags & NEW_FLAG_I_CARE_ABOUT)
> > >>     /* Kernel does support the known_flags extension and does
> > >>        support the feature I care about.  */
> > >> 
> > >> We could hide this in the clone3 wrapper for glibc if we start out with
> > >> a struct clone_args that has this member.
> > >
> > > So the kernel semantics I suggested but when the kernel does not support
> > > it have and doesn't set it have glibc set this?
> > 
> > Exactly.
> > 
> > > Yeah, that sounds like a good idea to me!
> > 
> > Good.  Please get this into the kernel. 8-)
> 
> Will do. :)

Ok, I need to be annoying once more. (As if there wasn't enough of that
already...)
So kernel-side a good friend of mine - Aleksa - and I are on the way to
push through _some_ standards... That very much includes how we check
for new extensions in syscalls. So we just discussed my earlier proposal
(Sorry for the lag the distance Australia to Germany makes things a bit
difficult.). One problem we'll have with adding a new member is that we
arguably need one for each additional flag member we add. That means
we'd need:

struct clone_args /* initial struct */ {
	__aligned_u64 flags;
	__aligned_u64 supported_flags;
}

and later

struct clone_args /* extended struct: new flags argument added */ {
	__aligned_u64 flags;
	__aligned_u64 supported_flags;
	__aligned_u64 flags2;
	__aligned_u64 supported_flags2;
}

which is not ideal. So we were thinking about a different protocol for
syscalls which embedd flags arguments in structs.

We'd introduce a new flag

#define SYSCALL_CHECK_FLAGS ((1 << 63)ULL)

that can be used by e.g. clone3() and openat2(). Then you can call the
syscall with:

struct clone_args args = {
	.flags |= SYSCALL_CHECK_FLAGS,
};
pid = clone3(&args, sizeof(args));

/* 
 * The kernel now sets all flags it supports in _all_ flags arguments
 * present in the struct.
 */

SYSCALL_CHECK_FLAGS is only valid in the initial flags argument not in
any added flags arguments.
If SYSCALL_CHECK_FLAGS is not supported then you'll get EINVAL and the
SYSCALL_CHECK_FLAGS flag is still present in the flags argument of the
struct. If it is supported it'll be masked out and all supported flag
bits will be set by the kernel in _all_ flag arguments that are present
in the structure.
Florian, thoughts?

Thanks!
Christian