linux-kernel - Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <nycvar.YFH.7.76.1809220936520.15880@cbobk.fhfr.pm>
Date:   Sat, 22 Sep 2018 09:38:50 +0200 (CEST)
From:   Jiri Kosina <jikos@...nel.org>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     "Schaufler, Casey" <casey.schaufler@...el.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        "Woodhouse, David" <dwmw@...zon.co.uk>,
        Andi Kleen <ak@...ux.intel.com>,
        Tim Chen <tim.c.chen@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection

On Wed, 19 Sep 2018, Peter Zijlstra wrote:

> > diff --git a/kernel/ptrace.c b/kernel/ptrace.c
> > index 5c5e7cb597cd..202a4d9c2af7 100644
> > --- a/kernel/ptrace.c
> > +++ b/kernel/ptrace.c
> > @@ -330,9 +330,7 @@ int __ptrace_may_access(struct task_struct *task, unsigned int mode)
> >                !ptrace_has_cap(mm->user_ns, mode))))
> >             return -EPERM;
> > 
> > -       if (!(mode & PTRACE_MODE_NOACCESS_CHK))
> > -               return security_ptrace_access_check(task, mode);
> > -       return 0;
> > +       return security_ptrace_access_check(task, mode);
> >  }
> > 
> >  bool ptrace_may_access(struct task_struct *task, unsigned int mode)
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index 161a4f29f860..30d21142e9fe 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -2215,7 +2215,12 @@ static int selinux_ptrace_access_check(struct task_struct *child,
> >  {
> >         u32 sid = current_sid();
> >         u32 csid = task_sid(child);
> > +       struct av_decision avd;
> > 
> > +       if (mode == PTRACE_MODE_IBPB)
> > +               return avc_has_perm_noaudit(&selinux_state, sid, csid,
> > +                                           SECCLASS_PROCESS, PROCESS__PTRACE,
> > +                                           0, &avd);
> >         if (mode & PTRACE_MODE_READ)
> >                 return avc_has_perm(&selinux_state,
> >                                     sid, csid, SECCLASS_FILE, FILE__READ, NULL);
> > 
> 
> As far as I can tell, this still has:
> 
> 	avc_has_perm_noaudit()
> 	  security_compute_av()
> 	    read_lock(&state->ss->policy_rwlock);
> 	  avc_insert()
> 	    spin_lock_irqsave();
> 	  avc_denied()
> 	    avc_update_node()
> 	      spin_lock_irqsave();
> 
> under the scheduler's raw_spinlock_t, which are invalid lock nestings.

Agreed. Therefore, if the current form (v6) of the patches is merged, the 
check before security_ptrace_access_check() should stay.

Once all the LSM callbacks are potentially audited, it could then go in 
second phase.

Is there anything else blocking v6 being merged? (and then Tim's set on 
top I guess, once the details are sorted out there).

Thanks,

-- 
Jiri Kosina
SUSE Labs