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

From: Lakshmi Ramasubramanian
Date: Mon Aug 24 2020 - 17:29:17 EST


On 8/24/20 1:01 PM, Ondrej Mosnacek wrote:
On Mon, Aug 24, 2020 at 9:30 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
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.

Yeah, and I'm experimenting with a patch on top of Stephen's RCU work
that would allow you to do this in a straightforward way without even
messing with the fsi->mutex. My patch may or may not be eventually
committed, but either way I'd recommend holding off on this for a
while until the dust settles around the RCU conversion.


I can make the SELinux\IMA changes in "selinux next branch" taking dependencies on Stephen's patches + relevant IMA patches.

Could you please let me know the URL to the "selinux next branch"?

thanks,
-lakshmi