On Tue, Aug 25, 2020 at 6:03 PM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > > On Tue, Aug 25, 2020 at 11:20 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote: > > > > Remove the security_policydb_len() calls from sel_open_policy() and > > instead update the inode size from the size returned from > > security_read_policy(). > > > > Since after this change security_policydb_len() is only called from > > security_load_policy(), remove it entirely and just open-code it there. > > > > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx> > > --- > > > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > > index 8381614627569..ec4570d6c38f7 100644 > > --- a/security/selinux/ss/services.c > > +++ b/security/selinux/ss/services.c > > @@ -3915,7 +3899,10 @@ int security_read_policy(struct selinux_state *state, > > if (!selinux_initialized(state)) > > return -EINVAL; > > > > - *len = security_policydb_len(state); > > + rcu_read_lock(); > > + policy = rcu_dereference(state->policy); > > + *len = policy->policydb.len; > > + rcu_read_unlock(); > > > > *data = vmalloc_user(*len); > > if (!*data) > > We don't actually need to take rcu_read_lock() here at all, and can > use rcu_dereference_check(..., 1) or rcu_deference_protected() because > the fsi->mutex is held. We should also fix the code immediately below > this diff context to not double dereference the policy pointer and > just reuse the already dereferenced pointer (also not requiring > rcu_read_lock). OK, I'll fix this up to just fetch the pointer as you suggest (and adapt the corresponding hunks in the other patches). -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.
