Re: [PATCH] SELinux: Measure state and hash of policy using IMA

[Stephen Smalley]

On Mon, Aug 24, 2020 at 2:13 PM Lakshmi Ramasubramanian
<nramas@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On 8/24/20 7:00 AM, Stephen Smalley wrote:
>
> >> +int security_read_policy_kernel(struct selinux_state *state,
> >> +                               void **data, size_t *len)
> >> +{
> >> +       int rc;
> >> +
> >> +       rc = security_read_policy_len(state, len);
> >> +       if (rc)
> >> +               return rc;
> >> +
> >> +       *data = vmalloc(*len);
> >> +       if (!*data)
> >> +               return -ENOMEM;
> >>
> >> +       return security_read_selinux_policy(state, data, len);
> >>   }
> >
> > See the discussion here:
> > https://lore.kernel.org/selinux/20200824113015.1375857-1-omosnace@xxxxxxxxxx/T/#t
> >
> > In order for this to be safe, you need to ensure that all callers of
> > security_read_policy_kernel() have taken fsi->mutex in selinuxfs and
> > any use of security_read_policy_len() occurs while holding the mutex.
> > Otherwise, the length can change between security_read_policy_len()
> > and security_read_selinux_policy() if policy is reloaded.
> >
>
> "struct selinux_fs_info" is available when calling
> security_read_policy_kernel() - currently in measure.c.
> Only "struct selinux_state" is.
>
> Is Ondrej's re-try approach I need to use to workaround policy reload issue?

No, I think perhaps we should move the mutex to selinux_state instead
of selinux_fs_info.  selinux_fs_info has a pointer to selinux_state so
it can then use it indirectly.  Note that your patches are going to
conflict with other ongoing work in the selinux next branch that is
refactoring policy load and converting the policy rwlock to RCU.