SELinux Archive on lore.kernel.org
 help / Atom feed
* [PATCH] selinux: Fix classmap for BPF
@ 2019-02-06  4:17 William A. Kennington III
  2019-02-06 14:04 ` Stephen Smalley
  0 siblings, 1 reply; 4+ messages in thread
From: William A. Kennington III @ 2019-02-06  4:17 UTC (permalink / raw)
  To: selinux; +Cc: William A. Kennington III

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.

Signed-off-by: William A. Kennington III <william@wkennington.com>
---
 security/selinux/include/classmap.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
index bd5fe0d3204a..7ff68a5e4c58 100644
--- a/security/selinux/include/classmap.h
+++ b/security/selinux/include/classmap.h
@@ -239,7 +239,7 @@ struct security_class_mapping secclass_map[] = {
 	{ "infiniband_endport",
 	  { "manage_subnet", NULL } },
 	{ "bpf",
-	  {"map_create", "map_read", "map_write", "prog_load", "prog_run"} },
+	  { "map_create", "map_read", "map_write", "prog_load", "prog_run", NULL } },
 	{ "xdp_socket",
 	  { COMMON_SOCK_PERMS, NULL } },
 	{ NULL }
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selinux: Fix classmap for BPF
  2019-02-06  4:17 [PATCH] selinux: Fix classmap for BPF William A. Kennington III
@ 2019-02-06 14:04 ` Stephen Smalley
  2019-02-06 14:33   ` Stephen Smalley
  0 siblings, 1 reply; 4+ messages in thread
From: Stephen Smalley @ 2019-02-06 14:04 UTC (permalink / raw)
  To: William A. Kennington III, selinux; +Cc: Paul Moore

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")

> 
> Signed-off-by: William A. Kennington III <william@wkennington.com>
> ---
>   security/selinux/include/classmap.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/security/selinux/include/classmap.h b/security/selinux/include/classmap.h
> index bd5fe0d3204a..7ff68a5e4c58 100644
> --- a/security/selinux/include/classmap.h
> +++ b/security/selinux/include/classmap.h
> @@ -239,7 +239,7 @@ struct security_class_mapping secclass_map[] = {
>   	{ "infiniband_endport",
>   	  { "manage_subnet", NULL } },
>   	{ "bpf",
> -	  {"map_create", "map_read", "map_write", "prog_load", "prog_run"} },
> +	  { "map_create", "map_read", "map_write", "prog_load", "prog_run", NULL } },

Should likely break the line to make checkpatch.pl happy:

$ ./scripts/checkpatch.pl -g HEAD
WARNING: line over 80 characters
#24: FILE: security/selinux/include/classmap.h:242:
+	  { "map_create", "map_read", "map_write", "prog_load", "prog_run", 
NULL } },


>   	{ "xdp_socket",
>   	  { COMMON_SOCK_PERMS, NULL } },
>   	{ NULL }
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selinux: Fix classmap for BPF
  2019-02-06 14:04 ` Stephen Smalley
@ 2019-02-06 14:33   ` Stephen Smalley
  2019-02-08  2:30     ` Paul Moore
  0 siblings, 1 reply; 4+ messages in thread
From: Stephen Smalley @ 2019-02-06 14:33 UTC (permalink / raw)
  To: William A. Kennington III, selinux; +Cc: Paul Moore

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.

> 
>>
>> Signed-off-by: William A. Kennington III <william@wkennington.com>
>> ---
>>   security/selinux/include/classmap.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/security/selinux/include/classmap.h 
>> b/security/selinux/include/classmap.h
>> index bd5fe0d3204a..7ff68a5e4c58 100644
>> --- a/security/selinux/include/classmap.h
>> +++ b/security/selinux/include/classmap.h
>> @@ -239,7 +239,7 @@ struct security_class_mapping secclass_map[] = {
>>       { "infiniband_endport",
>>         { "manage_subnet", NULL } },
>>       { "bpf",
>> -      {"map_create", "map_read", "map_write", "prog_load", 
>> "prog_run"} },
>> +      { "map_create", "map_read", "map_write", "prog_load", 
>> "prog_run", NULL } },
> 
> Should likely break the line to make checkpatch.pl happy:
> 
> $ ./scripts/checkpatch.pl -g HEAD
> WARNING: line over 80 characters
> #24: FILE: security/selinux/include/classmap.h:242:
> +      { "map_create", "map_read", "map_write", "prog_load", "prog_run", 
> NULL } },
> 
> 
>>       { "xdp_socket",
>>         { COMMON_SOCK_PERMS, NULL } },
>>       { NULL }
>>
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] selinux: Fix classmap for BPF
  2019-02-06 14:33   ` Stephen Smalley
@ 2019-02-08  2:30     ` Paul Moore
  0 siblings, 0 replies; 4+ messages in thread
From: Paul Moore @ 2019-02-08  2:30 UTC (permalink / raw)
  To: Stephen Smalley; +Cc: William A. Kennington III, selinux

On Wed, Feb 6, 2019 at 9:33 AM Stephen Smalley <sds@tycho.nsa.gov> 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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-06  4:17 [PATCH] selinux: Fix classmap for BPF William A. Kennington III
2019-02-06 14:04 ` Stephen Smalley
2019-02-06 14:33   ` Stephen Smalley
2019-02-08  2:30     ` Paul Moore

SELinux Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/selinux/0 selinux/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 selinux selinux/ https://lore.kernel.org/selinux \
		selinux@vger.kernel.org selinux@archiver.kernel.org
	public-inbox-index selinux


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.selinux


AGPL code for this site: git clone https://public-inbox.org/ public-inbox