Re: [PATCH 0/5] selinux: minor cleanup suggestions

[Stephen Smalley <sds@tycho.nsa.gov>]

On 09/25/2015 06:34 PM, Rasmus Villemoes wrote:
> A few random things I stumbled on.
>
> While I'm pretty sure of the change in 1/5, I'm also confused, because
> the doc for the reverse security_sid_to_context state that
> @scontext_len is set to "the length of the string", which one would
> normally interpret as being what strlen() would give (i.e., without
> the \0). However, security_sid_to_context_core clearly includes the \0
> in the return value, and I think callers rely on that.

It is historical; originally security_context_to_sid() required 
@scontext to be NUL-terminated and @scontext_len to include the NUL byte 
in the length, and security_sid_to_context() returned a NUL-terminated 
@scontext and included the NUL byte in the returned length.  However, 
when we switched SELinux to using xattrs rather than its own persistent 
label mapping, security_context_to_sid() was changed to accept contexts 
that did not already include the NUL because setfattr did not consider 
the NUL to be part of the attribute value for strings.  So presently it 
accepts either form, although we prefer them to be NUL-terminated and 
canonicalize them to that form before returning to userspace.

>
> Rasmus Villemoes (5):
>    selinux: introduce security_context_str_to_sid
>    selinux: remove pointless cast in selinux_inode_setsecurity()
>    selinux: use kmemdup in security_sid_to_context_core()
>    selinux: use kstrdup() in security_get_bools()
>    selinux: use sprintf return value
>
>   security/selinux/hooks.c            | 14 +++++---------
>   security/selinux/include/security.h |  2 ++
>   security/selinux/selinuxfs.c        | 26 +++++++++-----------------
>   security/selinux/ss/services.c      | 22 +++++++++-------------
>   4 files changed, 25 insertions(+), 39 deletions(-)
>