On 8/19/20 10:54 AM, Ondrej Mosnacek wrote: > 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://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_selinux_20200818194311.30018-2D1-2Dstephen.smalley.work-40gmail.com_&d=DwIBaQ&c=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc&r=oO5HuGEGxznA2F3djiiYxmxxWQonw0h6Sks-BEoB4ys&m=3n3GB_y_9DEclfBuCK3Cw5Qzkdet94qyGeoAhAr_EOE&s=CiXlpSOQIfnydwIcZDscD2fyYtDOdRquHhTWR1vWZRk&e= > >> 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. How then should they be synced? Self rcu:ed? > >> +}; >> + >> +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. >
