diff mbox series

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

Message ID 20230420160910.809091-2-teo.coupriediaz@arm.com
State Accepted
Headers show
Series setpgid: Test EPERM error paths more reliably | expand

Commit Message

Teo Couprie Diaz April 20, 2023, 4:09 p.m. UTC
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(-)

Comments

Li Wang April 21, 2023, 7:21 a.m. UTC | #1
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
>
>
Cyril Hrubis April 21, 2023, 8:04 a.m. UTC | #2
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.
diff mbox series

Patch

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)