Re: [PATCH 08/10] LSM: SafeSetID: add read handler

[Jann Horn <jannh@google.com>]

On Wed, Apr 10, 2019 at 7:27 PM Kees Cook <keescook@chromium.org> wrote:
> On Wed, Apr 10, 2019 at 9:56 AM Micah Morton <mortonm@chromium.org> wrote:
> >
> > From: Jann Horn <jannh@google.com>
> >
> > For debugging a running system, it is very helpful to be able to see what
> > policy the system is using. Add a read handler that can dump out a copy of
> > the loaded policy.
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
> > Signed-off-by: Micah Morton <mortonm@chromium.org>
> > ---
> >  security/safesetid/lsm.h        |  3 +++
> >  security/safesetid/securityfs.c | 38 +++++++++++++++++++++++++++++++--
> >  2 files changed, 39 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/safesetid/lsm.h b/security/safesetid/lsm.h
> > index 4a34f558d964..9380329fe30a 100644
> > --- a/security/safesetid/lsm.h
> > +++ b/security/safesetid/lsm.h
> > @@ -17,6 +17,7 @@
> >  #include <linux/types.h>
> >  #include <linux/uidgid.h>
> >  #include <linux/hashtable.h>
> > +#include <linux/refcount.h>
> >
> >  /* Flag indicating whether initialization completed */
> >  extern int safesetid_initialized;
> > @@ -41,7 +42,9 @@ struct setuid_rule {
> >
> >  struct setuid_ruleset {
> >         DECLARE_HASHTABLE(rules, SETID_HASH_BITS);
> > +       char *policy_str;
> >         struct rcu_head rcu;
> > +       refcount_t refcount;
> >  };
>
> refcount seems like overkill? Why not just use the spinlock? Neither
> read nor write are "fast path".

You can't copy to userspace under a spinlock or under RCU. But we
could change policy_update_lock to a mutex and hold that across the
policy read.