Messages in this thread | | | From | "Schaufler, Casey" <> | Subject | RE: [PATCH RFC v2 3/5] LSM: Security module checking for side-channel dangers | Date | Mon, 20 Aug 2018 15:31:22 +0000 |
| |
> -----Original Message----- > From: Jann Horn [mailto:jannh@google.com] > Sent: Friday, August 17, 2018 4:53 PM > To: Schaufler, Casey <casey.schaufler@intel.com> > Cc: Kernel Hardening <kernel-hardening@lists.openwall.com>; kernel list > <linux-kernel@vger.kernel.org>; linux-security-module <linux-security- > module@vger.kernel.org>; selinux@tycho.nsa.gov; Hansen, Dave > <dave.hansen@intel.com>; Dock, Deneen T <deneen.t.dock@intel.com>; > kristen@linux.intel.com; Arjan van de Ven <arjan@linux.intel.com> > Subject: Re: [PATCH RFC v2 3/5] LSM: Security module checking for side- > channel dangers > > On Sat, Aug 18, 2018 at 12:17 AM Casey Schaufler > <casey.schaufler@intel.com> wrote: > > > > From: Casey Schaufler <cschaufler@localhost.localdomain> > > > > The sidechannel LSM checks for cases where a side-channel > > attack may be dangerous based on security attributes of tasks. > > This includes: > > Effective UID of the tasks is different > > Capablity sets are different > > Tasks are in different namespaces > > An option is also provided to assert that task are never > > to be considered safe. This is high paranoia, and expensive > > as well. > > > > Signed-off-by: Casey Schaufler <casey.schaufler@intel.com> > [...] > > +#ifdef CONFIG_SECURITY_SIDECHANNEL_UIDS > > +static int safe_by_uid(struct task_struct *p) > > +{ > > + const struct cred *ccred = current_real_cred(); > > + const struct cred *pcred = get_task_cred(p); > > + > > + /* > > + * Credential checks. Considered safe if: > > + * UIDs are the same > > + */ > > + if (ccred != pcred && ccred->euid.val != pcred->euid.val) > > + return -EACCES; > > + return 0; > > +} > > This function looks bogus. get_task_cred() bumps the refcount on the > returned cred struct pointer, but you don't drop it. You probably want > to use something that doesn't fiddle with the refcount at all here to > avoid cacheline bouncing - possibly a raw rcu_dereference_protected() > if there are no better helpers. > > Same thing for the other get_task_cred() calls further down in the patch.
Thanks. Looks like I whacked out v2 a bit hastily.
| |