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

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

On Wed, Oct 09, 2019 at 01:56:15PM +0200, Florian Weimer wrote:
> * Christian Brauner:
> 
> > On Wed, Oct 09, 2019 at 01:04:03PM +0200, Florian Weimer wrote:
> >> * Christian Brauner:
> >> 
> >> > I've been thinking about two things how to do this:
> >> > - mask the flags that the kernel does not support
> >> 
> >> That doesn't look fully backwards-compatible to me.  The argument isn't
> >> currently read/write, is it?  It would work for us though.
> >> 
> >> > - add another argument to struct clone_args that is "known_flags"
> >> >   when the syscall returns it'll be set to all the flags this kernel
> >> >   knows about
> >> 
> >> This needs some sort of protocol to detect whether the argument was
> >> updated.  I suppose we could define CLONE3_INITIALLY_SUPPORTED_FLAGS
> >> with all the flag bits currently supported and tell developers to
> >> initialize struct clone_args with:
> >> 
> >>   .known_flags = CLONE3_INITIALLY_SUPPORTED_FLAGS,
> >
> > That won't work. Older kernels will verify that parts of the struct that
> > are not known are set to 0.
> 
> Good point.
> 
> > I wonder, what is stopping you from
> >
> > struct clone args args = {
> > 	.known_flags = 0,
> > };
> >
> > pid_t pid = clone3(&args, sizeof(args));
> > if (pid < 0)
> > 	return -1;
> >
> > ######### kernel code ############
> > /* on a kernel that is aware of known_flags */
> > kargs->known_flags = CLONE3_SUPPORTED_FLAGS;
> > ##################################
> >
> > if (!args.known_flags)
> > 	/* kernel doesn't not support the known_flags extension */
> >
> > if (args.known_flags & NEW_FLAG_I_CARE_ABOUT)
> > 	/* 
> > 	 * kernel does support the known_flags extension and does
> > 	 * support the feature I care about 
> > 	 */
> 
> 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?
Yeah, that sounds like a good idea to me!

Thanks!
Christian