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] [day] [month] [year] [list]

Message-ID: <aWp2pZqZ1mJ2zFLN@1wt.eu>
Date: Fri, 16 Jan 2026 18:34:29 +0100
From: Willy Tarreau <w@....eu>
To: Stephen Smalley <stephen.smalley.work@...il.com>
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 11:58:30AM -0500, Stephen Smalley wrote:
> On Fri, Jan 16, 2026 at 10:30 AM Willy Tarreau <w@....eu> wrote:
> > 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.

OK so that was the trickiest to spot (comments are mine above).

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

I'm fine with this.

> Happy to split it out into its own line and move after the mls_*()
> funciton if it would be easier to read.

I think that what's mostly missing is a comment before the construction
explaining the expected output format, and another comment saying that
the +1 cover the following delimiter for the first fields, and the
trailing zero for the last one, and that mls_*() include the needeed
delimiter. That way the calculation would look like it obviously
matches expectations.

Thanks for taking the time to explain, now we're certain there's no
issue and next time we get this report again we won't doubt anymore.

Willy