Re: [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID

Alejandro Colomar
2023-08-12T17:51:48Z

[-- Attachment #1.1: Type: text/plain, Size: 3266 bytes --]

On 2023-08-12 19:48, Alejandro Colomar wrote:
> Hello Sargun,
>
> On 2023-08-10 04:26, Sargun Dhillon wrote:
>> It appears like the documentation is based on out of date information in
>> regards to using CLONE_NEWPID and CLONE_PARENT together.
>>
>> For example, in this test program, one can see that it works:
>>
>> static pid_t sys_clone3(struct clone_args *args)
>> {
>> fflush(stdout);
>> fflush(stderr);
>> return syscall(__NR_clone3, args, sizeof(*args));
>> }
>>
>> int main() {
>> struct clone_args args = {
>> .flags = CLONE_PARENT | CLONE_NEWPID,
>> };
>> int ret;
>>
>> printf("The main program is running with pid: %d, and ppid: %d\n", getpid(), getppid());
>> ret = sys_clone3(&args);
>> assert(ret != -1);
>> if (ret == 0) {
>> printf("This is the child, running with pid: %d, and ppid: %d\n", getpid(), getppid());
>> _exit(0);
>
> Do we really need _exit(3)? Why not exit(3)? There are no atexit(3)
> or on_exit(3) handlers registered, so the only difference I expect is
> the flushing of stdio(3) streams, which _exit(3) doesn't perform but
> exit(3) does. So exit(3) seems more appropriate, isn't it?
>
>> }
>>
>> return 0;
>> }
>
> Thanks for the example program! It helps a lot with the review. :)
>
>>
>> This test program (successfully) outputs:
>> The main program is running with pid: 648411, and ppid: 648397
>> This is the child, running with pid: 1, and ppid: 0

Does this depend on any recent kernel version? In my system,
the assertion fails.

$ cat clone.c
#include <assert.h>
#include <linux/sched.h>
#include <sched.h>
#include <stdio.h>
#include <sys/syscall.h>
#include <unistd.h>

static pid_t
sys_clone3(struct clone_args *args)
{
fflush(stdout);
fflush(stderr);
return syscall(SYS_clone3, args, sizeof(*args));
}

int
main(void)
{
int ret;
struct clone_args args = { .flags = CLONE_PARENT | CLONE_NEWPID, };

printf("main program: pid: %d, and ppid: %d\n", getpid(), getppid());
ret = sys_clone3(&args);
assert(ret != -1);
if (ret == 0) {
printf("This is the child, running with pid: %d, and ppid: %d\n", getpid(), getppid());
_exit(0);
}

return 0;
}

$ cc -Wall -Wextra clone.c
$ ./a.out
main program: pid: 18783, and ppid: 18703
a.out: clone.c:24: main: Assertion `ret != -1' failed.
Aborted

>>
>> Signed-off-by: Sargun Dhillon <sargun@sargun.me>
>> ---
>> man2/clone.2 | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/man2/clone.2 b/man2/clone.2
>> index 4c5b4ac6b..96ad24b95 100644
>> --- a/man2/clone.2
>> +++ b/man2/clone.2
>> @@ -736,9 +736,7 @@ Only a privileged process
>> can employ
>> .BR CLONE_NEWPID .
>> This flag can't be specified in conjunction with
>> -.B CLONE_THREAD
>> -or
>> -.BR CLONE_PARENT .
>> +.B CLONE_THREAD.
>
> You'll need BR here, and the space before the period; otherwise,
> the period will be in bold, which we don't want (as it's not part
> of the identifier).
>
> Thanks,
> Alex
>
>> .TP
>> .B CLONE_NEWUSER
>> (This flag first became meaningful for
>

--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]