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: 7029 bytes --]

Hi Günther,

> Date: 2026-10-23 19:57:16+0200
> From: Günther Noack <gnoack3000@gmail.com>
>
> Hello Alejandro!
> 
> On Fri, Oct 23, 2026 at 11:57:41AM +0200, Alejandro Colomar wrote:
> > > Date: 2026-10-21 23:22:53+0200
> > > From: Günther Noack <gnoack3000@gmail.com>
> > >
> > > Reword the description of SCM_SECURITY to not be specific to SELinux.
> > > 
> > > The security context is populated in scm_passec() in linux.git
> > > net/core/scm.c, through the security_secid_to_secctx() LSM hook, which
> > > is currently implemented by SELinux, Smack and AppArmor.
> > > 
> > > Also clarify the properties of the returned string:
> > > 
> > > * There is no NAME_MAX limit on the length; SELinux contexts can be
> > >   longer than that.  If the control buffer is too small, put_cmsg()
> > >   truncates the data and sets MSG_CTRUNC.
> > > 
> > > * Whether the string is null-terminated also depends on the LSM:
> > >   SELinux includes the null byte in the length, but smack_to_secctx()
> > >   in security/smack/smack_lsm.c uses strlen() and doesn't.
> > 
> > ...
> 
> Ah, I can see that this is confusing; let me reformulate that without
> mentioning this strlen() implementation detail.  Smack returns a
> buffer with a given size *without* including the null byte, roughly
> like this (simplified):
> 
>   int len = strlen(skp->smk_known);
>   /* populate */
>   cp->context = skp->smk_known;
>   cp->len = len;    <-- does not span the null byte
> 
> Reformulating to:
> 
>   ...but smack_to_secctx() in security/smack/smack_lsm.c sends a
>   security context that does not end in a null byte.
> 
> > > Fixes: ffad6a017f91 (2018-07-28; "unix.7: Document SCM_SECURITY ancillary data")
> > > Cc: Michael Kerrisk <mtk.manpages@gmail.com>
> > > Cc: Kuniyuki Iwashima <kuniyu@google.com>
> > > Assisted-by: LLM
> > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > ---
> > > 
> > > Notes:
> > >     This feature is also available in other LSMs.  Unfortunately, they
> > >     implemented it inconsistently and that complicates the description of
> > >     how string boundaries are handled.  Linux does indeed neither
> > >     guarantee that this terminates with a NUL byte, nor that NAME_MAX is
> > >     large enough to hold it.
> > 
> > ...
> > 
> > >     
> > >     This is the verbose variant, but I am open for suggestions regarding
> > >     what level of detail is appropriate here.
> > > 
> > >  man/man7/unix.7 | 25 +++++++++++++++++++------
> > >  1 file changed, 19 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/man/man7/unix.7 b/man/man7/unix.7
> > > index ded82768322b..4abd216d19c7 100644
> > > --- a/man/man7/unix.7
> > > +++ b/man/man7/unix.7
> > > @@ -317,7 +317,7 @@ and returned as the result of
> > >  is an integer boolean flag.
> > >  .TP
> > >  .B SO_PASSSEC
> > > -Enables receiving of the SELinux security label of the peer socket
> > > +Enables receiving of the security label of the peer socket
> > >  in an ancillary message of type
> > >  .B SCM_SECURITY
> > >  (see below).
> > > @@ -544,13 +544,26 @@ message, the
> > >  option must be enabled on the socket.
> > >  .TP
> > >  .B SCM_SECURITY
> > > -Receive the SELinux security context (the security label)
> > > -of the peer socket.
> > > -The received ancillary data is a null-terminated string containing
> > > +Receive the security context (the security label)
> > > +of the peer socket,
> > > +as provided by the active Linux Security Module (LSM),
> > > +such as SELinux or Smack.
> > > +The received ancillary data is a string containing
> > >  the security context.
> > > -The receiver should allocate at least
> > > +Whether the string includes a terminating null byte
> > > +depends on the LSM;
> > 
> > This seems to contradict the commit message.  The commit message says
> > that the string is terminated, but that the reported length might or
> > might not be off-by-one (i.e., not count or count the '\0').
> >
> > 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.

> 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.


Have a lovely night!
Alex

> 
> –Günther

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

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