[LTP] [PATCH] setpgid02: Use pid_max as PGID for EPERM

[Li Wang]

On Wed, Apr 19, 2023 at 6:59 PM Cyril Hrubis <chrubis@suse.cz> wrote:

> Hi!
> > 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 a different session
> > and generating an EPERM error.
>
> So I suppose that we hit slightly different condition in the kernel:
>
>         if (pgid != pid) {
>                 struct task_struct *g;
>
>                 pgrp = find_vpid(pgid);
>                 g = pid_task(pgrp, PIDTYPE_PGID);
>                 if (!g || task_session(g) != task_session(group_leader))
>                         goto out;
>         }
>
> Previously we were supposed to hit the task_session(g) !=
> task_session(group_leader) now we hit !g.
>

Ah, yes, thanks for pointing out the difference from the kernel layer.



>
> Also I guess that the only way to hit the second part of the condition
> is to actually open and initialize a pty so that we have a process
> outside of the current session.
>

+1, this sounds correct way to reach that branch.
We can add one more item in the tcase struct to cover it.



>
> The patch itself looks okay, but we should at least update the
> documentation comment such as:
>
> diff --git a/testcases/kernel/syscalls/setpgid/setpgid02.c
> b/testcases/kernel/syscalls/setpgid/setpgid02.c
> index 4b63afee8..68b663633 100644
> --- a/testcases/kernel/syscalls/setpgid/setpgid02.c
> +++ b/testcases/kernel/syscalls/setpgid/setpgid02.c
> @@ -13,8 +13,8 @@
>   * - 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.
>   */
>
> And ideally write a test for the second case as well.
>
> --
> Cyril Hrubis
> chrubis@suse.cz
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>
>

-- 
Regards,
Li Wang