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

[Florian Weimer <fweimer at redhat dot com>]

* 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.

Thanks,
Florian