Re: [PATCH 1/2] xen/build-id: Introduce is_gnu_build_id()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.11.2026 17:25, Andrew Cooper wrote:
> On 04/11/2026 2:49 pm, Jan Beulich wrote:
>> On 04.11.2026 14:51, Andrew Cooper wrote:
>>> Broken out of the subsequent change for legibility reasons.  It will be used
>>> to replace xen_build_id_check().
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
>>> CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
>>> CC: Michal Orzel <michal.orzel@xxxxxxx>
>>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>>> CC: Julien Grall <julien@xxxxxxx>
>>> CC: Roger Pau Monné <roger@xxxxxxxxxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> ---
>>>  xen/common/version.c         | 33 +++++++++++++++++++++++++++++++++
>>>  xen/include/xen/elfstructs.h |  2 ++
>>>  xen/include/xen/version.h    |  2 ++
>>>  3 files changed, 37 insertions(+)
>>>
>>> diff --git a/xen/common/version.c b/xen/common/version.c
>>> index 80e7a9b981cf..7eb8a0221d82 100644
>>> --- a/xen/common/version.c
>>> +++ b/xen/common/version.c
>>> @@ -114,6 +114,37 @@ void print_version(void)
>>>  /* Defined in linker script. */
>>>  extern const Elf_Note __note_gnu_build_id_start[], 
>>> __note_gnu_build_id_end[];
>>>  
>>> +/*
>>> + * Check whether @n is a GNU Build ID note.
>>> + *
>>> + * We check that:
>>> + *  - type is NT_GNU_BUILD_ID
>>> + *  - name is GNU\0
>>> + *  - desc is non-empty and exactly covering the rest of the buffer
>>> + *
>>> + * If this returns true, the contents starts at n + NT_GNU_HDR_SIZE.
>>> + */
>>> +bool is_gnu_build_id(const Elf_Note *n, unsigned int len)
>>> +{
>>> +    const char *name;
>>> +
>>> +    if ( len <= NT_GNU_HDR_SIZE )
>>> +        return false;
>>> +
>>> +    if ( n->type != NT_GNU_BUILD_ID ||
>>> +         n->namesz != 4 )
>>> +        return false;
>>> +
>>> +    name = ELFNOTE_NAME(n);
>>> +    if ( strncmp(name, "GNU", 4) != 0 )
>>> +        return false;
>>> +
>>> +    if ( len != n->descsz + NT_GNU_HDR_SIZE )
>>> +        return false;
>> Isn't this too strict a check? Build-IDs can, iirc, take different formats
>> (yes, this is under our control, but still), and I don't think there's a
>> guarantee that all current and future ones are coming without any padding.
>> IOW I think len wants comparing against the rounded up sum.
> 
> Hmm.  There should be no padding included by the __note_gnu_build_id_end
> marker, but there may be in the section size used by livepatching. 
> Still, there must not be any rounding.
> 
> is_gnu_build_id() needs to return 0 for failure, and n->descsz for
> success, and this check needs to become len > n->descsz +
> NT_GNU_HDR_SIZE in order to tolerate tail padding.

The caller holding n in its hands, it can use n->descsz without you returning
it from here, i.e. the function could remain a boolean predicate, couldn't it?

> The caller cannot subtract NT_GNU_HDR_SIZE from the input length,
> because otherwise the padding would be considered part of the buildid.

Right.

Jan