Re: [PATCH 4/7] man/man7/unix.7: SCM_SECURITY: Generalize to other LSMs, not just SELinux

[Alejandro Colomar <alx@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 6650 bytes --]


Hi Casey,

> Date: 2026-10-27 09:32:57-0700
> From: Casey Schaufler <casey@schaufler-ca.com>
>
[...]
> >>>> This text, however, is saying that in some cases there might not be
> >>>> a '\0' at all.
> >>> The TL;DR is that the byte after the returned buffer's declared length
> >>> can't be trusted to be 0 - it is undefined and can be clobbered.
> >> Huh!  This is to me a bug.  Would you mind reporting them this as a bug
> >> and CCing me?  I'd rather get this fixed in the kernel than documented
> >> as broken.
> >>
> >> Especially, if they use strlen(3) internally, this sounds very much like
> >> a kernel bug, but even if not, it is clearly a kernel bug.
> >>
> >> In any case, if we were to document this bug, we should not use the term
> >> string, because anything not containing a '\0' is not a string, by
> >> definition.
> > Casey, would you be able to confirm whether this constitutes a Smack
> > bug or is intentional?
> 
> Inclusion of a null byte for C string interfaces has been a topic of
> debate since about 1980. At the time this code was written my understanding
> was that the data SCM_SECURITY returned could not be assumed to be a
> C string. Hence, the size of the data, not the size of a string. The
> null byte isn't considered a part of the string, as reported by strlen().
> 
> So, the existing behavior is intentional, but I'm willing to have it
> considered a bug if (as of today) everyone thinks the data should include
> a null byte.

Yup, I think the issue of strings has more or less settled towards using
C strings everywhere, with other formats --such as fixed-width buffers
from utmp(5)-- being phased out.  Most of libc takes C strings, and so
using C strings everywhere is safer, as it avoids having to manually
handle the bytes.

> > The TL;DR is that we came across the function smack_to_secctx() in
> > smack_lsm.c, which passes a Smack label string in a buffer without a
> > terminating null byte.  This is inconsistent with what SELinux and
> > AppArmor do.
> >
> > Before we start explaining so many corner cases in the man pages, I
> > wanted to check -- is this something that Smack should fix, by
> > including the zero byte?
> >
> > I tried to double check the Smack userspace utilities but couldn't
> > find any use of SCM_SECURITY there.  (Is the feature unused?)
> 
> The Smack userspace utilities (github.com://smackteam/smack.git)
> represent a limited Smack system implementation. SCM_SECURITY is
> used in at least a couple of important embedded and enterprise
> implementations.
> 
> >>> I have not read the full kernel implementation now, but it can already
> >>> be seen in userspace that that adjacent byte can belong to a different
> >>> data structue:
> >>>
> >>>
> >>> ## How control messages are represented
> >>>
> >>> The security context is returned to user space as a cmsghdr structure,
> >>> as described in cmsg(3), where it is returned as a buffer of length
> >>> cmsg_len:
> >>>
> >>> struct cmsghdr {
> >>> 	size_t cmsg_len;  /* data byte count including header */
> >>>         /* some other fields */
> >>>
> >>> 	/* followed by
> >>>            unsigned char cmsg_data[]; */
> >>> };
> >>>
> >>> Reading beyond the declared cmsg_len is undefined, and callers don't
> >>> get any guarantees about the byte following the csmg_data[] buffer.
> >>>
> >>>
> >>> ## Iterating from userspace through multiple control messages
> >>>
> >>> Iterating through the cmsgs is done with the API described in cmsg(3),
> >>> CMSG_NXTHDR().  This is implemented as an inlined function in
> >>> /usr/include/bits/socket.h.  The function does a bunch of validation
> >>> logic and then ends in this rather simple advancement logic:
> >>>
> >>>   __cmsg = (struct cmsghdr *) ((unsigned char *) __cmsg
> >>>                                + CMSG_ALIGN (__cmsg->cmsg_len));
> >>>   return __cmsg;
> >>>
> >>> So, the cmsgs with their cmsg_data[] buffers are next to each other in
> >>> memory.  They are aligned, but that still means that the byte after
> >>> cmsg_data[] might be part of the next cmsg header or beyond the list
> >>> of control messages.
> >>>
> >>> ---
> >>>
> >>>
> >>> The more I think about this, the more unsure I am that this is even
> >>> worth documenting.  In the end, anyone reading and interpreting this
> >>> data probably needs to read the corresponding LSM anyway, and trailing
> >>> null bytes are probably the least of their worries...?
> >> Not only that it's worth documenting, but this might be a vulnerability.
> >>> Maybe it would be better to just say something like "The format of the
> >>> security context is determined by the LSM responsible for it".  WDYT?
> >> I'm unwilling to accept a nonstring as a valid format, unless very good
> >> reasons are stated.  And if those reasons are stated as "that ship
> >> sailed long ago", the way to document this might very well be BUGS.
> > Point taken and agreed that it would be better to call it a "buffer"
> > rather than a "string" when it does not have the null terminator.
> >
> > It does seem that this is used also outside of UNIX Domain Sockets:
> > IP_PASSSEC(2const) says:
> >
> >   The security context returned in the SCM_SECURITY ancillary message
> >   is of the same format as the one described in SO_PEERSEC(2const).
> >
> > And looking this format up in SO_PEERSEC(2const), we find:
> >
> >   The security context string may include a terminating null character
> >   in the returned length, but is not guaranteed to do so: a security
> >   context "foo" might be represented as either {'f','o','o'} of length
> >   3 or {'f','o','o','\0'} of length 4, which are considered to be
> >   interchangeable.  The string is printable, does not contain non-
> >   terminating null characters, and is in an unspecified encoding (in
> >   particular, it is not guaranteed to be ASCII or UTF-8).
> >
> > So it seems that these "buffer" (not "string") semantics exist there
> > as well.  Maybe it is intentional then that it is treated as a buffer
> > rather than a string?
> 
> I'm easy. If there is a strong desire to make SCM_SECURITY a string
> Smack can be changed easily enough. 

Yup.  I think it'd be good to have guarantees that the label can be
passed to strlen(3).  Whether the reported size includes the '\0' or
not, that's less important, but being able to pass the buffer to
strlen(3) and any other string functions is essential for safety, IMO.

Thank you very much!


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]