On Wed, Aug 19, 2020 at 10:32 AM peter enderborg <peter.enderborg@xxxxxxxx> wrote: > From 8184ea3648b18718fdb460a30dfc7f848b7bc6a2 Mon Sep 17 00:00:00 2001 > From: Peter Enderborg <peter.enderborg@xxxxxxxx> > Date: Wed, 19 Aug 2020 10:20:28 +0200 > Subject: [RFC PATCH] selinux: Use call_rcu for policydb and booleans > > This patch adds call_rcu that moves sycronize out > out call path. In the callback we can no call > cond_resched so they have to be remvoed. Did you notice Stephen's comment about vfree() in the v1 patch [1]? That probably also needs addressing, and after that you'll likely end up with something very similar to the v1 patch anyway. [1] https://lore.kernel.org/selinux/20200818194311.30018-1-stephen.smalley.work@xxxxxxxxx/ > > Signed-off-by: Peter Enderborg <peter.enderborg@xxxxxxxx> > --- > security/selinux/ss/policydb.c | 6 ----- > security/selinux/ss/services.c | 43 ++++++++++++++++++++++++++++++---- > 2 files changed, 39 insertions(+), 10 deletions(-) > [...] > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c > index ba9347517e5b..11eff3a98ef8 100644 > --- a/security/selinux/ss/services.c > +++ b/security/selinux/ss/services.c > @@ -2184,11 +2184,29 @@ static void selinux_notify_policy_change(struct selinux_state *state, > selinux_xfrm_notify_policyload(); > } > > +struct deprecated_policy { > + struct selinux_policy *policy; > + int partial; > + struct rcu_head rcu; Why not just add these fields to struct selinux_policy directly? Then you can avoid the speculative allocation of struct deprecated_policy. > +}; > + > +void policy_reclaim(struct rcu_head *rp) > +{ > + struct deprecated_policy *dep = container_of(rp, struct dep_policy, rcu); > + > + if (dep->partial) > + selinux_policy_cond_free(dep->policy); > + else > + selinux_policy_free(dep->policy); > + kfree(dep); > +} > + > void selinux_policy_commit(struct selinux_state *state, > struct selinux_policy *newpolicy) > { > struct selinux_policy *oldpolicy; > u32 seqno; > + struct deprecated_policy *dep; > > /* > * NOTE: We do not need to take the rcu read lock > @@ -2231,8 +2249,16 @@ void selinux_policy_commit(struct selinux_state *state, > } > > /* Free the old policy */ > - synchronize_rcu(); > - selinux_policy_free(oldpolicy); > + /* if cant alloc we need to it the slow way */ > + dep = kzalloc(sizeof(struct deprecated_policy), GFP_KERNEL); > + if (dep) { > + dep->policy = oldpolicy; > + dep->partial = 0; > + call_rcu(&dep->rcu, policy_reclaim); > + } else { > + synchronize_rcu(); > + selinux_policy_free(oldpolicy); > + } > > /* Notify others of the policy change */ > selinux_notify_policy_change(state, seqno); [...] -- Ondrej Mosnacek Software Engineer, Platform Security - SELinux kernel Red Hat, Inc.
