On Wed, Feb 6, 2019 at 9:33 AM Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:
> On 2/6/19 9:04 AM, Stephen Smalley wrote:
> > On 2/5/19 11:17 PM, William A. Kennington III wrote:
> >> Entries in the secclass_map are expexted to be null terminated. The BPF
> >> entry was added without the NULL terminating and incosistent formatting.
> >> This patch cleans that up.
> >
> > Thanks. A few minor nits:
> >
> > A couple of spelling errors above (expected, inconsistent). Also, per
> > Documentation/process/submitting-patches.rst, rather than say "This
> > patch cleans that up", say "Clean that up" or similar.
> >
> > Can add a:
> > Fixes: ec27c3568a34c7f ("selinux: bpf: Add selinux check for eBPF
> > syscall operations")
>
> Although I guess there isn't really a bug here; this is just a
> consistency / style issue. secclass_map[] is defined as:
>
> struct security_class_mapping {
> const char *name;
> const char *perms[sizeof(u32) * 8 + 1];
> };
>
> struct security_class_mapping secclass_map[];
>
> So even if you were to omit the terminating NULL from each permission
> list, any remaining slots in the perms array should be initialized to
> NULL automatically. We only truly need the explicit NULL terminator to
> end the class list.
To clarify this William, if you can make the changes that Stephen
suggested I think this is still a worthwhile patch, even if it doesn't
really fix a bug in the code. I would add some comments in the patch
description indicating that this is more about consistency than fixing
an error.
--
paul moore
www.paul-moore.com
