linux-kernel - Re: SELinux lead to soft lockup when pid 1 proceess reap child

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Jan 2017 19:29:15 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     yangshukui <yangshukui@...wei.com>
Cc:     selinux@...ho.nsa.gov, linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        "Guohanjun (Hanjun Guo)" <guohanjun@...wei.com>,
        "'Qiang Huang'" <h.huangqiang@...wei.com>,
        Lizefan <lizefan@...wei.com>, "miaoxie (A)" <miaoxie@...wei.com>,
        Zhangdianfang <zhangdianfang@...wei.com>, paul@...l-moore.com,
        sds@...ho.nsa.gov, eparis@...isplace.org,
        james.l.morris@...cle.com, ebiederm@...ssion.com,
        serge.hallyn@...ntu.com
Subject: Re: SELinux lead to soft lockup when pid 1 proceess reap child

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