Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Sat, 22 Sep 2018 09:38:50 +0200 (CEST)
From:   Jiri Kosina <jikos@kernel.org>
To:     Peter Zijlstra <peterz@infradead.org>
cc:     "Schaufler, Casey" <casey.schaufler@intel.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        Andrea Arcangeli <aarcange@redhat.com>,
        "Woodhouse, David" <dwmw@amazon.co.uk>,
        Andi Kleen <ak@linux.intel.com>,
        Tim Chen <tim.c.chen@linux.intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
Subject: Re: [PATCH v6 0/3] Harden spectrev2 userspace-userspace protection
In-Reply-To: <20180919154828.GJ24124@hirez.programming.kicks-ass.net>
Message-ID: <nycvar.YFH.7.76.1809220936520.15880@cbobk.fhfr.pm>
References: <nycvar.YFH.7.76.1809101119280.15880@cbobk.fhfr.pm> <nycvar.YFH.7.76.1809121055070.15880@cbobk.fhfr.pm> <99FC4B6EFCEFD44486C35F4C281DC6732144EA58@ORSMSX107.amr.corp.intel.com> <20180919154828.GJ24124@hirez.programming.kicks-ass.net>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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