Re: [PATCH v8 1/3] Add an internal wrapper for clone, clone2 and clone3

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 13/07/2021 15:54, Adhemerval Zanella wrote:
> 
> 
> On 01/06/2021 11:55, H.J. Lu wrote:
>> The clone3 system call provides a superset of the functionality of clone
>> and clone2.  It also provides a number of API improvements, including
>> the ability to specify the size of the child's stack area which can be
>> used by kernel to compute the shadow stack size when allocating the
>> shadow stack.  Add:
>>
>> extern int __clone_internal (struct clone_args *__cl_args,
>> 			     int (*__func) (void *__arg), void *__arg);
>>
>> to provide an abstract interface for clone, clone2 and clone3.
>>
>> 1. Simplify stack management for thread creation by passing both stack
>> base and size to create_thread.
>> 2. Consolidate clone vs clone2 differences into a single file.
>> 3. Call __clone3 if HAVE_CLONE3_WAPPER is defined.  If __clone3 returns
>> -1 with ENOSYS, fall back to clone or clone2.
>> 4. Use only __clone_internal to clone a thread.  Since the stack size
>> argument for create_thread is now unconditional, always pass stack size
>> to create_thread.
>> 5. Enable the public clone3 wrapper in the future after it has been
>> added to all targets.
>>
>> NB: Sandbox should return ENOSYS on clone3 if it is rejected:
>>
>> https://bugs.chromium.org/p/chromium/issues/detail?id=1213452#c5
> 
> LGTM with just an suggestion below.  Also chromium also has fixed it,
> so although it wouldn't be able to fully handled clone3, at least
> it won't brick a 2.34 glibc.
> 
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

I forgot to add that I think it would be useful to reference the kernel
version and commit which actually added the clone3 syscall on the
commit log.