linux-kernel - Re: Suspected off-by-one in context_struct_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEjxPJ6xyngQaWGiih+LkJ=C7yvJYMEp7Nr92tzC+hVJy0R3PQ@mail.gmail.com>
Date: Fri, 16 Jan 2026 11:58:30 -0500
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: Willy Tarreau <w@....eu>
Cc: Christian Göttsche <cgzones@...glemail.com>, 
	Paul Moore <paul@...l-moore.com>, security@...nel.org, selinux@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: Suspected off-by-one in context_struct_to_string()

On Fri, Jan 16, 2026 at 10:30 AM Willy Tarreau <w@....eu> wrote:
>
> Hi Stephen,
>
> On Fri, Jan 16, 2026 at 10:12:15AM -0500, Stephen Smalley wrote:
> > On Fri, Jan 16, 2026 at 3:16 AM Christian Göttsche
> > <cgzones@...glemail.com> wrote:
> > >
> > > On Thu, 15 Jan 2026 at 21:20, Willy Tarreau <w@....eu> wrote:
> > > >
> > > > Hello,
> > > >
> > > > we've received a suspected vulnerability report on the kernel security
> > > > list, that was clearly generated by AI and really not clear at all on
> > > > the root causes nor impacts. We first dismissed it and it kept coming
> > > > back a few times. I'm not pasting it because it's more confusing than
> > > > interesting, though I can pass it to the maintainers if desired. I'm
> > > > also purposely *not* CCing the reporter, as the address changed a few
> > > > times, and once you respond you receive a new copy of the same report.
> > > > Clearly this bot deserves a bit more tuning.
> > > >
> > > > The report claimed that the call to mls_compute_context_len() didn't
> > > > properly reflect the size needed by mls_sid_to_context() due to an
> > > > off-by-one that would result in the trailing zero being written too far.
> > > > Initially we thought that was wrong since there are +1 everywhere in
> > > > all lengths calculation in the function. But revisiting it today made
> > > > us realize that this indeed seems to be true: the +1 that are everywhere
> > > > are in fact due to the surrounding delimiters, and the first one that
> > > > appeared to be the one accounting for the trailing zero was in fact
> > > > for the starting colon.
> > > >
> > > > In context_struct_to_string(), we have this:
> > > >
> > > >         *scontext_len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1;
> > > >         *scontext_len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1;
> > > >         *scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1;
> > >
> > > I think this +1 from the type name length covers the trailing NUL
> > > byte, since mls_compute_context_len() and mls_sid_to_context() cover
> > > the one byte space for the separating colon between type and optional
> > > MLS component.
> >
> > That is correct. The MLS portion is conditional on whether MLS is
> > enabled; hence, the +1 for the type length computation includes the
> > terminating NUL,
> > and then the mls_compute_context_len() starts at 1 for the leading ":"
> > if MLS is enabled. The code could admittedly be clearer but I don't
> > believe this is a bug.
>
> OK and I, too, think that mls_compute_context_len() stands by what
> its name implies. But then *who* is responsible in all this chain
> for allocating the room for the trailing zero that is being appended
> at the end ?
>
> Or is this the last +1 of this block maybe ?
>
>      *scontext_len += strlen(sym_name(p, SYM_USERS, context->user - 1)) + 1; // ':'
>      *scontext_len += strlen(sym_name(p, SYM_ROLES, context->role - 1)) + 1; // ':'
>      *scontext_len += strlen(sym_name(p, SYM_TYPES, context->type - 1)) + 1; // \0 ?
>
> I'm asking because nothing is really clear, and if it happens to work as
> intended, it's not super clear why.

Yes, it is that last +1. Historically, originally the MLS support was
a Kconfig option and the entire
mls_*() parts were compiled out altogether if MLS was disabled. In
that situation, the context
ended with the type name and thus counting the NUL aka '\0' byte there
made sense. Later the MLS
support was changed to be dynamically determined at policy load time,
but that function still returns 0
if MLS is disabled so the NUL byte is still counted in the type name
length computation above.
Happy to split it out into its own line and move after the mls_*()
funciton if it would be easier to read.