Re: Suspected off-by-one in context_struct_to_string()

[Willy Tarreau <w@1wt.eu>]

On Fri, Jan 16, 2026 at 09:16:10AM +0100, Christian Göttsche wrote:
> On Thu, 15 Jan 2026 at 21:20, Willy Tarreau <w@1wt.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.

Sorry if I'm not clear, but my point is that above each strlen()+1
seems to serve as the length of the text + its colon delimiter, so
it covers useful chars and excludes the trailing zero, which is fine.

> >         *scontext_len += mls_compute_context_len(p, context);

Here it does exactly the same.

> >
> > *scontext_len is initialized to zero, is increased by the length of each
> > appended string + delimiter, and used as-is in kmalloc() a few lines later:

So now we're allocating an area of the number of useful chars, not
counting the trailing zero.

> >         scontextp = kmalloc(*scontext_len, GFP_ATOMIC);
> >
> > then filled by sprintf() then mls_sid_to_context():
> >
> >         scontextp += sprintf(scontextp, "%s:%s:%s",
> >                 sym_name(p, SYM_USERS, context->user - 1),
> >                 sym_name(p, SYM_ROLES, context->role - 1),
> >                 sym_name(p, SYM_TYPES, context->type - 1));
> >
> >         mls_sid_to_context(p, context, &scontextp);
> >
> > And finally the trailing zero is appended:
> >
> >         *scontextp = 0;

Yet we're emitting it.

At least that's how I read it.

Willy