Re: [PATCH 0/2] tools/symbols: fix oversized names and token expansion

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

Hi Jan,

Thank you for the review.
Sorry for the late reply.

On Thu, Oct 8, 2026 at 10:35 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 07.10.2026 20:11, Mykola Kvach wrote:
> > Patch 1 fixes the first bug. In SYSV input, local symbols are prefixed
> > with the source file name ("filename#symbol"). A long absolute build
> > path can therefore create a name longer than 127 bytes, even when the
> > symbol itself is short. A debug Xen build on an Orange Pi 5 (RK3588S)
> > crashed during the boot self-tests: symbols_lookup_by_name() expanded
> > such a name into its 128-byte buffer and overwrote the saved frame
> > pointer and return address. The patch rejects retained names longer than
> > KSYM_NAME_LEN during the build and prints the offending name.
> > Readable --xensyms maps are not restricted. Nothing changes for names
> > of at most 127 bytes.
> >
> > A build from a long absolute source path can now fail at link time.
>
> This isn't acceptable imo. Omitting such a symbol is a far less severe
> action (see below for another option).
>
> As a more general remark: We've inherited this tool from Linux. Any
> issues we find want cross checking there. If fixed there, taking their
> change is often preferable over making our own. To cover overly long
> symbols names, they have a check in read_symbol(). As we add the
> filename for static symbols there as well, I don't see why we couldn't
> similarly have the check there.

Agreed. v2 will report the name and skip the symbol, as Linux does,
and the check is in read_symbol().

>
> Furthermore, if the main issue is with long filenames, for static
> symbols an option would be to replace the filename prefix with some
> surrogate (either a shortened form of the path, or something like
> "...#").

I'd prefer to skip the symbol here. A surrogate such as "...#" makes
different static symbols look the same, and --error-dup can then fail
in livepatch builds.

>
> Yet further, the problem of (long or not) absolute path names is to
> be solved differently anyway. Not just for reproducible builds we want
> to omit them altogether. There are two separate proposals [1], [2];
> what's missing is a decision which way to go. (Both are pending a
> resubmission, yet perhaps not just me but also Marek is hesitant to
> do so when the decision which route to go is still pending.)

Agreed, both proposals address the absolute-path problem.
In my arm64 assembly test, both remove the absolute path from
the file symbol. With -fdebug-prefix-map alone, the path stays.

>
> > On x86, in-tree builds use relative paths and are not affected. An
> > out-of-tree build with the default config fails if the absolute path of
> > xen/ is longer than 78 bytes; the longest such name is
> > kexec_reloc.S#compatibility_mode.
> >
> > Patch 2 fixes the second bug. In the compressed stream, each symbol is
> > stored as [type][name]. A compression token can therefore expand to
> > the type byte plus a full 127-byte name (128 bytes), and write_src()
> > adds a terminating NUL. Its local buffer was 128 bytes, one byte too
> > small. The patch allocates KSYM_NAME_LEN + 2 bytes (type + name +
> > NUL). This bug can trigger with fully valid names, but only with a very
> > small symbol table; real symbol tables do not hit it. The new size
> > relies on the name limit from patch 1.
>
> Linux'es write_src() doesn't even add 1 to KSYM_NAME_LEN. The +1 there
> went away in 2.6.23, commit 9281acea6a36 ("kallsyms: make KSYM_NAME_LEN
> include space for trailing '\0'"). (Later KSYM_NAME_LEN was bumped to
> 512, btw.) Might we not want to follow that approach? (There then still
> looks to be an off-by-one due to the type that's being prefixed to all
> symbols. If that's indeed the case, a patch may want sending also for
> Linux.)

Yes. v2 will make KSYM_NAME_LEN include the trailing NUL.
Names are still limited to 127 bytes. The token buffer has
one extra byte for the type.

I also reproduced the overflow in Linux 7.4-rc2 with ASan.
Expanding the type, a 511-byte name and the NUL needs 513 bytes,
but the buffer has 512. I have not prepared a Linux patch.

Best regards,
Mykola



>
> Jan
>
> [1] https://lists.xen.org/archives/html/xen-devel/2025-09/msg00311.html
> [2] https://lists.xen.org/archives/html/xen-devel/2025-09/msg00362.html
>