Seriously, could someone explain why do we need the security_task_wait()
hook at all?
On 01/09, Oleg Nesterov wrote:
>
> On 01/09, yangshukui wrote:
> >
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -3596,6 +3596,9 @@ static int selinux_task_kill(struct task_struct *p,
> > struct siginfo *info,
> >
> > static int selinux_task_wait(struct task_struct *p)
> > {
> > + if (pid_vnr(task_tgid(current)) == 1){
> > + return 0;
>
> this check is not really correct, it can be a sub-thread... Doesn't matter,
> please see below.
>
> > + }
> > return task_has_perm(p, current, PROCESS__SIGCHLD);
> > }
> > It work but it permit pid 1 process to reap child without selinux check. Can
> > we have a better way to handle this problem?
>
> I never understood why security_task_wait() should deny to reap a child. But
> since it can we probably want some explicit "the whole namespace goes away"
> check.
> We could use, say, PIDNS_HASH_ADDING but I'd suggest something like a trivial
> change
> below for now.
>
> Eric, what do you think?
>
> Oleg.
>
> diff --git a/security/security.c b/security/security.c
> index f825304..1330b4e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1027,6 +1027,9 @@ int security_task_kill(struct task_struct *p, struct
> siginfo *info,
>
> int security_task_wait(struct task_struct *p)
> {
> + /* must be the exiting child reaper */
> + if (unlikely(current->flags & PF_EXITING))
> + return 0;
> return call_int_hook(task_wait, 0, p);
> }
>
