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

[Alejandro Colomar <alx@kernel.org>]

[-- 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 --]