On Wed, Aug 26, 2020 at 4:12 PM Stephen Smalley
<stephen.smalley.work@xxxxxxxxx> wrote:
> On Wed, Aug 26, 2020 at 9:59 AM Ondrej Mosnacek <omosnace@xxxxxxxxxx> wrote:
> >
> > The policycap array in struct selinux_state is redundant and can be
> > substituted by calling security_policycap_supported().
> >
> > Signed-off-by: Ondrej Mosnacek <omosnace@xxxxxxxxxx>
> > ---
>
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index 7cc2f7486c18f..e82a2cfe171f3 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -2113,30 +2113,6 @@ bad:
> > return 0;
> > }
> >
> > -static void security_load_policycaps(struct selinux_state *state,
> > - struct selinux_policy *policy)
> > -{
> > - struct policydb *p;
> > - unsigned int i;
> > - struct ebitmap_node *node;
> > -
> > - p = &policy->policydb;
> > -
> > - for (i = 0; i < ARRAY_SIZE(state->policycap); i++)
> > - state->policycap[i] = ebitmap_get_bit(&p->policycaps, i);
> > -
> > - for (i = 0; i < ARRAY_SIZE(selinux_policycap_names); i++)
> > - pr_info("SELinux: policy capability %s=%d\n",
> > - selinux_policycap_names[i],
> > - ebitmap_get_bit(&p->policycaps, i));
> > -
> > - ebitmap_for_each_positive_bit(&p->policycaps, node, i) {
> > - if (i >= ARRAY_SIZE(selinux_policycap_names))
> > - pr_info("SELinux: unknown policy capability %u\n",
> > - i);
> > - }
> > -}
> > -
>
> Two requests:
> 1. Can you do a little benchmarking to confirm that calling
> security_policycap_supported() each time doesn't cause any significant
> overheads? Networking benchmark might be of interest.
I tried to sample a simple `ping -f -i 0 -c 5000000 127.0.0.1` with
perf and indeed security_policycap_supported() now makes up about half
the time spent in some hooks (selinux_socket_sock_rcv_skb(),
selinux_ip_postroute()), mainly because of ebitmap_get_bit() it seems.
I'll try moving the array to policydb and using it in
security_policycap_supported() instead of the bitmap.
>
> 2. Can you retain the logging of the policy capability values? Just
> drop the first part of the function and rename it e.g.
> security_log_policycaps().
Sure, somehow I failed to notice those prints...
--
Ondrej Mosnacek
Software Engineer, Platform Security - SELinux kernel
Red Hat, Inc.
