[LTP] [PATCH v2 0/2] setpgid: Test EPERM error paths more reliably

[LTP] [PATCH v2 0/2] setpgid: Test EPERM error paths more reliably

[Teo Couprie Diaz]

The current EPERM case in setpgid02 can fail in systems running the
login shell as init (PID 1).

Change the failing case to fail due to an invalid PGID instead of
a different session, but add a case in setpgid03 to continue
testing this error path in the kernel.

This is because setpgid03 already has the necessary scaffolding ready
by forking and synchronizing with a child that does setsid().


CI Build: https://github.com/Teo-CD/ltp/actions/runs/4756354740

Teo Couprie Diaz (2):
  setpgid02: Use pid_max as PGID for EPERM
  setpgid03: Add test for PGID in different session

 testcases/kernel/syscalls/setpgid/setpgid02.c | 14 +++++++-------
 testcases/kernel/syscalls/setpgid/setpgid03.c |  4 ++++
 2 files changed, 11 insertions(+), 7 deletions(-)

-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/2] setpgid02: Use pid_max as PGID for EPERM

[Teo Couprie Diaz]

In some simple systems (like Busybox), the login shell might be run
as init (PID 1).
This leads to a case where LTP is run in the same session as init,
thus setpgid is allowed to the PGID of init which results in a test fail.
Indeed, the test retrieves the PGID of init to try and generate EPERM.

Instead get the PGID we use to generate EPERM from the kernel pid_max.
It should not be used by any process, guaranteeing an invalid PGID
and generating an EPERM error.

Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
---
 testcases/kernel/syscalls/setpgid/setpgid02.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/testcases/kernel/syscalls/setpgid/setpgid02.c b/testcases/kernel/syscalls/setpgid/setpgid02.c
index 4b63afee8..b380d7df4 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid02.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid02.c
@@ -13,15 +13,15 @@
  * - EINVAL when given pgid is less than 0.
  * - ESRCH when pid is not the calling process and not a child of
  * the calling process.
- * - EPERM when an attempt was made to move a process into a process
- * group in a different session.
+ * - EPERM when an attempt was made to move a process into a nonexisting
+ * process group.
  */
 
 #include <errno.h>
 #include <unistd.h>
 #include "tst_test.h"
 
-static pid_t pgid, pid, ppid, init_pgid;
+static pid_t pgid, pid, ppid, inval_pgid;
 static pid_t negative_pid = -1;
 
 static struct tcase {
@@ -31,7 +31,7 @@ static struct tcase {
 } tcases[] = {
 	{&pid, &negative_pid, EINVAL},
 	{&ppid, &pgid, ESRCH},
-	{&pid, &init_pgid, EPERM}
+	{&pid, &inval_pgid, EPERM}
 };
 
 static void setup(void)
@@ -41,10 +41,10 @@ static void setup(void)
 	pgid = getpgrp();
 
 	/*
-	 * Getting pgid of init/systemd process to use it as a
-	 * process group from a different session for EPERM test
+	 * pid_max would not be in use by another process and guarantees that
+	 * it corresponds to an invalid PGID, generating EPERM.
 	 */
-	init_pgid = SAFE_GETPGID(1);
+	SAFE_FILE_SCANF("/proc/sys/kernel/pid_max", "%d\n", &inval_pgid);
 }
 
 static void run(unsigned int n)
-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] setpgid02: Use pid_max as PGID for EPERM

[Li Wang]

On Fri, Apr 21, 2023 at 12:09 AM Teo Couprie Diaz <teo.coupriediaz@arm.com>
wrote:

> In some simple systems (like Busybox), the login shell might be run
> as init (PID 1).
> This leads to a case where LTP is run in the same session as init,
> thus setpgid is allowed to the PGID of init which results in a test fail.
> Indeed, the test retrieves the PGID of init to try and generate EPERM.
>
> Instead get the PGID we use to generate EPERM from the kernel pid_max.
> It should not be used by any process, guaranteeing an invalid PGID
> and generating an EPERM error.
>
> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
> ---
>  testcases/kernel/syscalls/setpgid/setpgid02.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid02.c
> b/testcases/kernel/syscalls/setpgid/setpgid02.c
> index 4b63afee8..b380d7df4 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid02.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid02.c
> @@ -13,15 +13,15 @@
>   * - EINVAL when given pgid is less than 0.
>   * - ESRCH when pid is not the calling process and not a child of
>   * the calling process.
> - * - EPERM when an attempt was made to move a process into a process
> - * group in a different session.
> + * - EPERM when an attempt was made to move a process into a nonexisting
> + * process group.
>   */
>
>  #include <errno.h>
>  #include <unistd.h>
>  #include "tst_test.h"
>
> -static pid_t pgid, pid, ppid, init_pgid;
> +static pid_t pgid, pid, ppid, inval_pgid;
>  static pid_t negative_pid = -1;
>
>  static struct tcase {
> @@ -31,7 +31,7 @@ static struct tcase {
>  } tcases[] = {
>         {&pid, &negative_pid, EINVAL},
>         {&ppid, &pgid, ESRCH},
> -       {&pid, &init_pgid, EPERM}
> +       {&pid, &inval_pgid, EPERM}
>  };
>
>  static void setup(void)
> @@ -41,10 +41,10 @@ static void setup(void)
>         pgid = getpgrp();
>
>         /*
> -        * Getting pgid of init/systemd process to use it as a
> -        * process group from a different session for EPERM test
> +        * pid_max would not be in use by another process and guarantees
> that
> +        * it corresponds to an invalid PGID, generating EPERM.
>          */
> -       init_pgid = SAFE_GETPGID(1);
> +       SAFE_FILE_SCANF("/proc/sys/kernel/pid_max", "%d\n", &inval_pgid);
>


I guess the '\n' is a typo added by accident,
after removing it then:

Reviewed-by: Li Wang <liwang@redhat.com>



>  }
>
>  static void run(unsigned int n)
> --
> 2.34.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/2] setpgid02: Use pid_max as PGID for EPERM

[Cyril Hrubis]

Hi!
> I guess the '\n' is a typo added by accident,
> after removing it then:

Actually it shouldn't matter, most of the sysfs files have newline at
the end so that you can just cat them and see the value in terminal
nicely.

$ cat /proc/sys/kernel/pid_max |xxd
00000000: 3332 3736 380a                           32768.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/2] setpgid03: Add test for PGID in different session

[Teo Couprie Diaz]

The current test in setpgid03 generates EPERM because the child is
a session leader, as it has called setsid().
EPERM can also happen by trying to change to a PGID in another session.
This was previously done in setpgid02, but it could fail on some systems.

setpgid03 provides a guaranteed way to generate this error by forking and
setsid() in the child, so add a test for it here.

Update the description to reflect this understanding.

Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
---
 testcases/kernel/syscalls/setpgid/setpgid03.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c b/testcases/kernel/syscalls/setpgid/setpgid03.c
index b23d662e9..9ce2603d8 100644
--- a/testcases/kernel/syscalls/setpgid/setpgid03.c
+++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
@@ -11,6 +11,8 @@
  *
  * Tests setpgid(2) errors:
  *
+ * - EPERM The process specified by pid must not be a session leader.
+ *
  * - EPERM The calling process, process specified by pid and the target
  *         process group must be in the same session.
  *
@@ -43,6 +45,8 @@ static void run(void)
 	TST_CHECKPOINT_WAIT(0);
 
 	TST_EXP_FAIL(setpgid(child_pid, getppid()), EPERM);
+	/* Child did setsid(), so its PGID is set to its PID. */
+	TST_EXP_FAIL(setpgid(0, child_pid), EPERM);
 
 	TST_CHECKPOINT_WAKE(0);
 
-- 
2.34.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/2] setpgid03: Add test for PGID in different session

[Li Wang]

On Fri, Apr 21, 2023 at 12:09 AM Teo Couprie Diaz <teo.coupriediaz@arm.com>
wrote:

> The current test in setpgid03 generates EPERM because the child is
> a session leader, as it has called setsid().
> EPERM can also happen by trying to change to a PGID in another session.
> This was previously done in setpgid02, but it could fail on some systems.
>
> setpgid03 provides a guaranteed way to generate this error by forking and
> setsid() in the child, so add a test for it here.
>
> Update the description to reflect this understanding.
>
> Signed-off-by: Teo Couprie Diaz <teo.coupriediaz@arm.com>
>

Reviewed-by: Li Wang <liwang@redhat.com>

---
>  testcases/kernel/syscalls/setpgid/setpgid03.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid03.c
> b/testcases/kernel/syscalls/setpgid/setpgid03.c
> index b23d662e9..9ce2603d8 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid03.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid03.c
> @@ -11,6 +11,8 @@
>   *
>   * Tests setpgid(2) errors:
>   *
> + * - EPERM The process specified by pid must not be a session leader.
> + *
>   * - EPERM The calling process, process specified by pid and the target
>   *         process group must be in the same session.
>   *
> @@ -43,6 +45,8 @@ static void run(void)
>         TST_CHECKPOINT_WAIT(0);
>
>         TST_EXP_FAIL(setpgid(child_pid, getppid()), EPERM);
> +       /* Child did setsid(), so its PGID is set to its PID. */
> +       TST_EXP_FAIL(setpgid(0, child_pid), EPERM);
>
>         TST_CHECKPOINT_WAKE(0);
>
> --
> 2.34.1
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/2] setpgid: Test EPERM error paths more reliably

[Cyril Hrubis]

Hi!
Both pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 0/2] setpgid: Test EPERM error paths more reliably

[Teo Couprie Diaz]

On 26/04/2023 12:43, Cyril Hrubis wrote:
> Hi!
> Both pushed, thanks.
>
Hi Cyril,
Thanks a lot !


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp