public inbox for linux-man@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID
@ 2023-08-10  2:26 Sargun Dhillon
  2023-08-12 17:48 ` Alejandro Colomar
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Sargun Dhillon @ 2023-08-10  2:26 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man

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);
	}

	return 0;
}

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

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.
 .TP
 .B CLONE_NEWUSER
 (This flag first became meaningful for
-- 
2.39.3


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID
  2023-08-10  2:26 [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID Sargun Dhillon
@ 2023-08-12 17:48 ` Alejandro Colomar
  2023-08-12 17:51   ` Alejandro Colomar
  2023-08-13 13:55 ` [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT Alejandro Colomar
  2023-08-13 14:14 ` [PATCH v4] " Alejandro Colomar
  2 siblings, 1 reply; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-12 17:48 UTC (permalink / raw)
  To: Sargun Dhillon; +Cc: linux-man


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

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID
  2023-08-12 17:48 ` Alejandro Colomar
@ 2023-08-12 17:51   ` Alejandro Colomar
  2023-08-12 19:05     ` John Watts
  0 siblings, 1 reply; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-12 17:51 UTC (permalink / raw)
  To: Sargun Dhillon; +Cc: linux-man


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID
  2023-08-12 17:51   ` Alejandro Colomar
@ 2023-08-12 19:05     ` John Watts
  2023-08-13  0:57       ` Sargun Dhillon
  0 siblings, 1 reply; 15+ messages in thread
From: John Watts @ 2023-08-12 19:05 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: Sargun Dhillon, linux-man

On Sat, Aug 12, 2023 at 07:51:43PM +0200, Alejandro Colomar wrote:
> Does this depend on any recent kernel version?  In my system,
> the assertion fails.
> 
> 
> $ cat clone.c 
> ...
> 
> $ 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

On my systerm I get the same result. strace says this:

clone3({flags=CLONE_PARENT|CLONE_NEWPID, exit_signal=0, stack=NULL, stack_size=0}, 88) = -1 EPERM (Operation not permitted)

However when running as root it works.

John.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID
  2023-08-12 19:05     ` John Watts
@ 2023-08-13  0:57       ` Sargun Dhillon
  2023-08-13 13:17         ` [PATCH v2] clone.2: Fix outdated " Alejandro Colomar
  0 siblings, 1 reply; 15+ messages in thread
From: Sargun Dhillon @ 2023-08-13  0:57 UTC (permalink / raw)
  To: John Watts; +Cc: Alejandro Colomar, linux-man

On Sat, Aug 12, 2023 at 1:05 PM John Watts <contact@jookia.org> wrote:
>
> On Sat, Aug 12, 2023 at 07:51:43PM +0200, Alejandro Colomar wrote:
> > Does this depend on any recent kernel version?  In my system,
> > the assertion fails.
> >
> >
> > $ cat clone.c
> > ...
> >
> > $ 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
It looks like this was added in
1f7f4dde5c945f41a7abc2285be43d918029ecc5 in v3.13.

>
> On my systerm I get the same result. strace says this:
>
> clone3({flags=CLONE_PARENT|CLONE_NEWPID, exit_signal=0, stack=NULL, stack_size=0}, 88) = -1 EPERM (Operation not permitted)
>
> However when running as root it works.
>
> John.
CLONE_NEWPID requires privileges.

See:

              Only a privileged process (CAP_SYS_ADMIN) can employ
              CLONE_NEWPID.  This flag can't be specified in conjunction
              with CLONE_THREAD or CLONE_PARENT.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2] clone.2: Fix outdated statement about CLONE_NEWPID
  2023-08-13  0:57       ` Sargun Dhillon
@ 2023-08-13 13:17         ` Alejandro Colomar
  2023-08-13 13:35           ` Serge E. Hallyn
  0 siblings, 1 reply; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 13:17 UTC (permalink / raw)
  To: linux-man, Sargun Dhillon, Serge Hallyn; +Cc: Alejandro Colomar, John Watts

From: Sargun Dhillon <sargun@sargun.me>

[TO += Serge]

Hi Sargun, Serge!

Sargun, I've slightly changed your patch, to keep the historical
information that in old versions of Linux this restriction was in place.
I also cleaned up the example program a little bit, and fixed the
formatting mistake I mentioned.  See scissor patch below.

However, reading the Linux commit that changed this, I had some doubts.
I've added Serge, in case he can remember the details.

Serge, I see that the commit message for (Linux) 1f7f4dde5c94 quotes
some email of yours, and mentions that the commit fixes a regression.
So my doubt is: was this restriction in place before v3.13 as a stable
thing, or was it only accidentally introduced temporarily and soon
fixed?  How should we document it?

Cheers,
Alex

---8<-----

It appears like the documentation is based on out of date information in
regards to using CLONE_NEWPID and CLONE_PARENT together.  Since Linux
v3.13, this restriction has been lifted.  See commit 1f7f4dde5c94
("fork:  Allow CLONE_PARENT after setns(CLONE_NEWPID)").

For example, in this test program, one can see that it works:

 #include <assert.h>
 #include <err.h>
 #include <linux/sched.h>
 #include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/syscall.h>
 #include <unistd.h>

static pid_t sys_clone3(struct clone_args *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);
	switch (ret) {
	case -1:
		err(EXIT_FAILURE, "clone3");
	case 0:
		printf("child: pid: %d, and ppid: %d\n", getpid(), getppid());
		exit(EXIT_SUCCESS);
	default:
		exit(EXIT_SUCCESS);
	}
}

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

This test program (successfully) outputs:

    # ./a.out
    main program: pid: 34663, and ppid: 34662
    child: pid: 1, and ppid: 0

Cowritten-by: Sargun Dhillon <sargun@sargun.me>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: John Watts <contact@jookia.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---

 man2/clone.2 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/man2/clone.2 b/man2/clone.2
index b91b71831..225fef86d 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -736,9 +736,11 @@ .SS The flags mask
 can employ
 .BR CLONE_NEWPID .
 This flag can't be specified in conjunction with
-.B CLONE_THREAD
-or
-.BR CLONE_PARENT .
+.BR CLONE_THREAD .
+Before Linux 3.13,
+this flag couldn't be specified in conjunction with
+.B CLONE_PARENT
+either.
 .TP
 .B CLONE_NEWUSER
 (This flag first became meaningful for
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] clone.2: Fix outdated statement about CLONE_NEWPID
  2023-08-13 13:17         ` [PATCH v2] clone.2: Fix outdated " Alejandro Colomar
@ 2023-08-13 13:35           ` Serge E. Hallyn
  2023-08-13 13:40             ` Alejandro Colomar
  0 siblings, 1 reply; 15+ messages in thread
From: Serge E. Hallyn @ 2023-08-13 13:35 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man, Sargun Dhillon, Serge Hallyn, John Watts

On Sun, Aug 13, 2023 at 03:17:27PM +0200, Alejandro Colomar wrote:
> From: Sargun Dhillon <sargun@sargun.me>
> 
> [TO += Serge]
> 
> Hi Sargun, Serge!
> 
> Sargun, I've slightly changed your patch, to keep the historical
> information that in old versions of Linux this restriction was in place.
> I also cleaned up the example program a little bit, and fixed the
> formatting mistake I mentioned.  See scissor patch below.
> 
> However, reading the Linux commit that changed this, I had some doubts.
> I've added Serge, in case he can remember the details.
> 
> Serge, I see that the commit message for (Linux) 1f7f4dde5c94 quotes
> some email of yours, and mentions that the commit fixes a regression.
> So my doubt is: was this restriction in place before v3.13 as a stable
> thing, or was it only accidentally introduced temporarily and soon
> fixed?  How should we document it?
> 
> Cheers,
> Alex
> 
> ---8<-----
> 
> It appears like the documentation is based on out of date information in
> regards to using CLONE_NEWPID and CLONE_PARENT together.  Since Linux
> v3.13, this restriction has been lifted.  See commit 1f7f4dde5c94
> ("fork:  Allow CLONE_PARENT after setns(CLONE_NEWPID)").
> 
> For example, in this test program, one can see that it works:
> 
>  #include <assert.h>
>  #include <err.h>
>  #include <linux/sched.h>
>  #include <sched.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <sys/syscall.h>
>  #include <unistd.h>
> 
> static pid_t sys_clone3(struct clone_args *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);
> 	switch (ret) {
> 	case -1:
> 		err(EXIT_FAILURE, "clone3");
> 	case 0:
> 		printf("child: pid: %d, and ppid: %d\n", getpid(), getppid());
> 		exit(EXIT_SUCCESS);
> 	default:
> 		exit(EXIT_SUCCESS);
> 	}
> }
> 
> static pid_t
> sys_clone3(struct clone_args *args)
> {
> 	fflush(stdout);
> 	fflush(stderr);
> 	return syscall(SYS_clone3, args, sizeof(*args));
> }
> 
> This test program (successfully) outputs:
> 
>     # ./a.out
>     main program: pid: 34663, and ppid: 34662
>     child: pid: 1, and ppid: 0
> 
> Cowritten-by: Sargun Dhillon <sargun@sargun.me>
> Cc: Serge Hallyn <serge@hallyn.com>
> Cc: John Watts <contact@jookia.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
> 
>  man2/clone.2 | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/man2/clone.2 b/man2/clone.2
> index b91b71831..225fef86d 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -736,9 +736,11 @@ .SS The flags mask
>  can employ
>  .BR CLONE_NEWPID .
>  This flag can't be specified in conjunction with
> -.B CLONE_THREAD
> -or
> -.BR CLONE_PARENT .
> +.BR CLONE_THREAD .
> +Before Linux 3.13,
> +this flag couldn't be specified in conjunction with
> +.B CLONE_PARENT

Well no, I don't think that's right.  That implies that the
CLONE_PARENT check was a long standing one.  In fact, the
point was that it was a regression introduced in

40a0d32d1 (Wed Sep 11 14:19:41 2013 -0700)

and then fixed in 1f7f4dde5 two months later.

> +either.
>  .TP
>  .B CLONE_NEWUSER
>  (This flag first became meaningful for
> -- 
> 2.40.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] clone.2: Fix outdated statement about CLONE_NEWPID
  2023-08-13 13:35           ` Serge E. Hallyn
@ 2023-08-13 13:40             ` Alejandro Colomar
  2023-08-13 13:53               ` Serge E. Hallyn
  0 siblings, 1 reply; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 13:40 UTC (permalink / raw)
  To: Serge E. Hallyn; +Cc: linux-man, Sargun Dhillon, John Watts


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

Hi Serge,

On 2023-08-13 15:35, Serge E. Hallyn wrote:
[...]

> 
> Well no, I don't think that's right.  That implies that the
> CLONE_PARENT check was a long standing one.  In fact, the
> point was that it was a regression introduced in
> 
> 40a0d32d1 (Wed Sep 11 14:19:41 2013 -0700)
> 
> and then fixed in 1f7f4dde5 two months later.

Thanks!  I'll rewrite the commit accordingly (more like Sargun's v1)
and include this info.

Cheers,
Alex

> 
>> +either.
>>  .TP
>>  .B CLONE_NEWUSER
>>  (This flag first became meaningful for
>> -- 
>> 2.40.1

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


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2] clone.2: Fix outdated statement about CLONE_NEWPID
  2023-08-13 13:40             ` Alejandro Colomar
@ 2023-08-13 13:53               ` Serge E. Hallyn
  0 siblings, 0 replies; 15+ messages in thread
From: Serge E. Hallyn @ 2023-08-13 13:53 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: Serge E. Hallyn, linux-man, Sargun Dhillon, John Watts

On Sun, Aug 13, 2023 at 03:40:36PM +0200, Alejandro Colomar wrote:
> Hi Serge,
> 
> On 2023-08-13 15:35, Serge E. Hallyn wrote:
> [...]
> 
> > 
> > Well no, I don't think that's right.  That implies that the
> > CLONE_PARENT check was a long standing one.  In fact, the
> > point was that it was a regression introduced in
> > 
> > 40a0d32d1 (Wed Sep 11 14:19:41 2013 -0700)
> > 
> > and then fixed in 1f7f4dde5 two months later.
> 
> Thanks!  I'll rewrite the commit accordingly (more like Sargun's v1)
> and include this info.

Thanks :)

-serge

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT
  2023-08-10  2:26 [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID Sargun Dhillon
  2023-08-12 17:48 ` Alejandro Colomar
@ 2023-08-13 13:55 ` Alejandro Colomar
  2023-08-13 14:03   ` Alejandro Colomar
  2023-08-13 14:36   ` Serge E. Hallyn
  2023-08-13 14:14 ` [PATCH v4] " Alejandro Colomar
  2 siblings, 2 replies; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 13:55 UTC (permalink / raw)
  To: linux-man; +Cc: Alejandro Colomar, Sargun Dhillon, Serge Hallyn, John Watts

From: Sargun Dhillon <sargun@sargun.me>

CLONE_NEWPID|CLONE_PARENT was only prohibited during a short period.
That prohibition was introduced in Linux 3.12, in commit 40a0d32d1eaf
("fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks"), but
was a regression, and was fixed in Linux 3.13, in commit 1f7f4dde5c94
("fork:  Allow CLONE_PARENT after setns(CLONE_NEWPID)").

In this test program, one can see that it works:

 #include <err.h>
 #include <linux/sched.h>
 #include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/syscall.h>
 #include <unistd.h>

static pid_t sys_clone3(struct clone_args *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);
	switch (ret) {
	case -1:
		err(EXIT_FAILURE, "clone3");
	case 0:
		printf("child: pid: %d, and ppid: %d\n", getpid(), getppid());
		exit(EXIT_SUCCESS);
	default:
		exit(EXIT_SUCCESS);
	}
}

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

This test program (successfully) outputs:

    # ./a.out
    main program: pid: 34663, and ppid: 34662
    child: pid: 1, and ppid: 0

Cowritten-by: Sargun Dhillon <sargun@sargun.me>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: John Watts <contact@jookia.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 man2/clone.2 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/man2/clone.2 b/man2/clone.2
index b91b71831..7d2dc2339 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -736,9 +736,7 @@ .SS The flags mask
 can employ
 .BR CLONE_NEWPID .
 This flag can't be specified in conjunction with
-.B CLONE_THREAD
-or
-.BR CLONE_PARENT .
+.BR CLONE_THREAD .
 .TP
 .B CLONE_NEWUSER
 (This flag first became meaningful for
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT
  2023-08-13 13:55 ` [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT Alejandro Colomar
@ 2023-08-13 14:03   ` Alejandro Colomar
  2023-08-13 14:36   ` Serge E. Hallyn
  1 sibling, 0 replies; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 14:03 UTC (permalink / raw)
  To: linux-man; +Cc: Sargun Dhillon, Serge Hallyn, John Watts


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

On 2023-08-13 15:55, Alejandro Colomar wrote:
[...]

> ---
>  man2/clone.2 | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/man2/clone.2 b/man2/clone.2
> index b91b71831..7d2dc2339 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -736,9 +736,7 @@ .SS The flags mask
>  can employ
>  .BR CLONE_NEWPID .
>  This flag can't be specified in conjunction with
> -.B CLONE_THREAD
> -or
> -.BR CLONE_PARENT .
> +.BR CLONE_THREAD .
>  .TP
>  .B CLONE_NEWUSER
>  (This flag first became meaningful for

Hmm, it seems this patch was incomplete.  This is a reminder that we
should specify 'Fixes: ...commit...' for these fixes as much as
possible.  Doing that, I found that the commit that it fixes did more
than that, and we need to fix those places too.  :)

I'll prepare a v4 reverting that patch entirely.

Cheers,
Alex


$ git show f00071920e
commit f00071920ec3ff5ed3ae2a8933259c535aae15a6
Author: Michael Kerrisk <mtk.manpages@gmail.com>
Date:   Tue Mar 5 09:55:39 2013 +0100

    clone.2: EINVAL if (CLONE_NEWUSER|CLONE_NEWPID) && (CLONE_THREAD|CLONE_PARENT)
    
    Signed-off-by: Michael Kerrisk <mtk.manpages@gmail.com>

diff --git a/man2/clone.2 b/man2/clone.2
index 5193e43af..e99b69b1e 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -407,7 +407,9 @@ .SH DESCRIPTION
 can employ
 .BR CLONE_NEWPID .
 This flag can't be specified in conjunction with
-.BR CLONE_THREAD .
+.BR CLONE_THREAD
+or
+.BR CLONE_PARENT .
 .TP
 .BR CLONE_NEWUSER
 (This flag first became meaningful for
@@ -441,6 +443,12 @@ .SH DESCRIPTION
 .\" Before Linux 2.6.29, it appears that only CAP_SYS_ADMIN was needed
 Starting with Linux 3.8,
 no privileges are needed to create a user namespace.
+
+.BR CLONE_NEWUSER
+cannot be specified in conjunction with
+.BR CLONE_NEWPID
+or
+.BR CLONE_PARENT .
 .TP
 .BR CLONE_NEWUTS " (since Linux 2.6.19)"
 If
@@ -895,10 +903,14 @@ .SH ERRORS
 .IR flags .
 .TP
 .B EINVAL
-Both
+One (or both) of
 .BR CLONE_NEWPID
-and
+or
+.BR CLONE_NEWUSER
+and one (or both) of
 .BR CLONE_THREAD
+or
+.BR CLONE_PARENT
 were specified in
 .IR flags .
 .TP


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


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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT
  2023-08-10  2:26 [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID Sargun Dhillon
  2023-08-12 17:48 ` Alejandro Colomar
  2023-08-13 13:55 ` [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT Alejandro Colomar
@ 2023-08-13 14:14 ` Alejandro Colomar
  2 siblings, 0 replies; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 14:14 UTC (permalink / raw)
  To: linux-man; +Cc: Alejandro Colomar, Sargun Dhillon, Serge Hallyn, John Watts

From: Sargun Dhillon <sargun@sargun.me>

CLONE_NEWPID|CLONE_PARENT was only prohibited during a short period.
That prohibition was introduced in Linux 3.12, in commit 40a0d32d1eaf
("fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks"), but
was a regression, and was fixed in Linux 3.13, in commit 1f7f4dde5c94
("fork:  Allow CLONE_PARENT after setns(CLONE_NEWPID)").

In this test program, one can see that it works:

 #include <err.h>
 #include <linux/sched.h>
 #include <sched.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/syscall.h>
 #include <unistd.h>

static pid_t sys_clone3(struct clone_args *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);
	switch (ret) {
	case -1:
		err(EXIT_FAILURE, "clone3");
	case 0:
		printf("child: pid: %d, and ppid: %d\n", getpid(), getppid());
		exit(EXIT_SUCCESS);
	default:
		exit(EXIT_SUCCESS);
	}
}

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

This test program (successfully) outputs:

    # ./a.out
    main program: pid: 34663, and ppid: 34662
    child: pid: 1, and ppid: 0

Fixes: f00071920ec3 ("clone.2: EINVAL if (CLONE_NEWUSER|CLONE_NEWPID) && (CLONE_THREAD|CLONE_PARENT)")
Cowritten-by: Sargun Dhillon <sargun@sargun.me>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: John Watts <contact@jookia.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 man2/clone.2 | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/man2/clone.2 b/man2/clone.2
index b91b71831..4a75b557b 100644
--- a/man2/clone.2
+++ b/man2/clone.2
@@ -729,23 +729,21 @@ .SS The flags mask
 For further information on PID namespaces, see
 .BR namespaces (7)
 and
 .BR pid_namespaces (7).
 .IP
 Only a privileged process
 .RB ( CAP_SYS_ADMIN )
 can employ
 .BR CLONE_NEWPID .
 This flag can't be specified in conjunction with
-.B CLONE_THREAD
-or
-.BR CLONE_PARENT .
+.BR CLONE_THREAD .
 .TP
 .B CLONE_NEWUSER
 (This flag first became meaningful for
 .BR clone ()
 in Linux 2.6.23,
 the current
 .BR clone ()
 semantics were merged in Linux 3.5,
 and the final pieces to make the user namespaces completely usable were
 merged in Linux 3.8.)
@@ -1310,32 +1308,37 @@ .SH ERRORS
 .B EINVAL
 Both
 .B CLONE_NEWIPC
 and
 .B CLONE_SYSVSEM
 were specified in the
 .I flags
 mask.
 .TP
 .B EINVAL
-One (or both) of
 .B CLONE_NEWPID
-or
-.B CLONE_NEWUSER
 and one (or both) of
 .B CLONE_THREAD
 or
 .B CLONE_PARENT
 were specified in the
 .I flags
 mask.
 .TP
+.B EINVAL
+.B CLONE_NEWUSER
+and
+.B CLONE_THREAD
+were specified in the
+.I flags
+mask.
+.TP
 .BR EINVAL " (since Linux 2.6.32)"
 .\" commit 123be07b0b399670a7cc3d82fef0cb4f93ef885c
 .B CLONE_PARENT
 was specified, and the caller is an init process.
 .TP
 .B EINVAL
 Returned by the glibc
 .BR clone ()
 wrapper function when
 .I fn
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT
  2023-08-13 13:55 ` [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT Alejandro Colomar
  2023-08-13 14:03   ` Alejandro Colomar
@ 2023-08-13 14:36   ` Serge E. Hallyn
  2023-08-13 14:37     ` Alejandro Colomar
  1 sibling, 1 reply; 15+ messages in thread
From: Serge E. Hallyn @ 2023-08-13 14:36 UTC (permalink / raw)
  To: Alejandro Colomar; +Cc: linux-man, Sargun Dhillon, Serge Hallyn, John Watts

On Sun, Aug 13, 2023 at 03:55:25PM +0200, Alejandro Colomar wrote:
> From: Sargun Dhillon <sargun@sargun.me>
> 
> CLONE_NEWPID|CLONE_PARENT was only prohibited during a short period.
> That prohibition was introduced in Linux 3.12, in commit 40a0d32d1eaf
> ("fork: unify and tighten up CLONE_NEWUSER/CLONE_NEWPID checks"), but
> was a regression, and was fixed in Linux 3.13, in commit 1f7f4dde5c94
> ("fork:  Allow CLONE_PARENT after setns(CLONE_NEWPID)").
> 
> In this test program, one can see that it works:
> 
>  #include <err.h>
>  #include <linux/sched.h>
>  #include <sched.h>
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <sys/syscall.h>
>  #include <unistd.h>
> 
> static pid_t sys_clone3(struct clone_args *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);
> 	switch (ret) {
> 	case -1:
> 		err(EXIT_FAILURE, "clone3");
> 	case 0:
> 		printf("child: pid: %d, and ppid: %d\n", getpid(), getppid());
> 		exit(EXIT_SUCCESS);
> 	default:
> 		exit(EXIT_SUCCESS);
> 	}
> }
> 
> static pid_t
> sys_clone3(struct clone_args *args)
> {
> 	fflush(stdout);
> 	fflush(stderr);
> 	return syscall(SYS_clone3, args, sizeof(*args));
> }
> 
> This test program (successfully) outputs:
> 
>     # ./a.out
>     main program: pid: 34663, and ppid: 34662
>     child: pid: 1, and ppid: 0
> 
> Cowritten-by: Sargun Dhillon <sargun@sargun.me>
> Cc: Serge Hallyn <serge@hallyn.com>

Reviewed-by: Serge Hallyn <serge@hallyn.com>

> Cc: John Watts <contact@jookia.org>
> Signed-off-by: Alejandro Colomar <alx@kernel.org>
> ---
>  man2/clone.2 | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/man2/clone.2 b/man2/clone.2
> index b91b71831..7d2dc2339 100644
> --- a/man2/clone.2
> +++ b/man2/clone.2
> @@ -736,9 +736,7 @@ .SS The flags mask
>  can employ
>  .BR CLONE_NEWPID .
>  This flag can't be specified in conjunction with
> -.B CLONE_THREAD
> -or
> -.BR CLONE_PARENT .
> +.BR CLONE_THREAD .
>  .TP
>  .B CLONE_NEWUSER
>  (This flag first became meaningful for
> -- 
> 2.40.1

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT
  2023-08-13 14:36   ` Serge E. Hallyn
@ 2023-08-13 14:37     ` Alejandro Colomar
  2023-08-13 14:40       ` Alejandro Colomar
  0 siblings, 1 reply; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 14:37 UTC (permalink / raw)
  To: Serge E. Hallyn, Sargun Dhillon; +Cc: linux-man, John Watts


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

Hi Sargun, Serge,

On 2023-08-13 16:36, Serge E. Hallyn wrote:
>> Cowritten-by: Sargun Dhillon <sargun@sargun.me>

Sargun, would you please sign the patch?

>> Cc: Serge Hallyn <serge@hallyn.com>
> 
> Reviewed-by: Serge Hallyn <serge@hallyn.com>

Thanks!

Cheers,
Alex

> 
>> Cc: John Watts <contact@jookia.org>
>> Signed-off-by: Alejandro Colomar <alx@kernel.org>
>> ---
-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT
  2023-08-13 14:37     ` Alejandro Colomar
@ 2023-08-13 14:40       ` Alejandro Colomar
  0 siblings, 0 replies; 15+ messages in thread
From: Alejandro Colomar @ 2023-08-13 14:40 UTC (permalink / raw)
  To: Serge E. Hallyn, Sargun Dhillon; +Cc: linux-man, John Watts


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

On 2023-08-13 16:37, Alejandro Colomar wrote:
> Hi Sargun, Serge,
> 
> On 2023-08-13 16:36, Serge E. Hallyn wrote:
>>> Cowritten-by: Sargun Dhillon <sargun@sargun.me>
> 
> Sargun, would you please sign the patch?
> 
>>> Cc: Serge Hallyn <serge@hallyn.com>
>>
>> Reviewed-by: Serge Hallyn <serge@hallyn.com>

Oops, I just realized that I accidentally pushed the patch to <kernel.org>
a few minutes ago.  I can't add your tag :(

Cheers,
Alex

> 
> Thanks!
> 
> Cheers,
> Alex
> 
>>
>>> Cc: John Watts <contact@jookia.org>
>>> Signed-off-by: Alejandro Colomar <alx@kernel.org>
>>> ---

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


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2023-08-13 14:40 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-10  2:26 [PATCH] clone.2: Fix the erroneous statement about CLONE_NEWPID Sargun Dhillon
2023-08-12 17:48 ` Alejandro Colomar
2023-08-12 17:51   ` Alejandro Colomar
2023-08-12 19:05     ` John Watts
2023-08-13  0:57       ` Sargun Dhillon
2023-08-13 13:17         ` [PATCH v2] clone.2: Fix outdated " Alejandro Colomar
2023-08-13 13:35           ` Serge E. Hallyn
2023-08-13 13:40             ` Alejandro Colomar
2023-08-13 13:53               ` Serge E. Hallyn
2023-08-13 13:55 ` [PATCH v3] clone.2: Fix erroneous statement about CLONE_NEWPID|CLONE_PARENT Alejandro Colomar
2023-08-13 14:03   ` Alejandro Colomar
2023-08-13 14:36   ` Serge E. Hallyn
2023-08-13 14:37     ` Alejandro Colomar
2023-08-13 14:40       ` Alejandro Colomar
2023-08-13 14:14 ` [PATCH v4] " Alejandro Colomar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox