[PATCH v2 1/1] readelf: invalid error message triggered when last tag is an empty string

[Jan Beulich]

On 19.06.2025 10:43, Matthieu Longo wrote:
> Disclaimer: this issue cannot occur with Object Attributes v1 (OAv1) because
> a value of '\0' (empty string) for a tag with a string value is considered
> as the default value for the attribute, and consequently is eliminated
> from the output object during the serialization.

Continuing from my earlier lack of understanding here: What "output object"
is this talking about? You're altering readelf, which only has an input
object (of unknown origin).

> An empty string is a valid value for a NTBS tag in both OAv1 and OAv2 [1]
> cases. However, contrarily to OAv1, a OAv2 subsection can be required and
> so, tags in this subsection might have to be present even if the value is
> the default. To comply with this requirement, the OAv2 serializer won't
> drop the default values.
> 
> In the case where a NTBS tag has for value '\0' and is last in the object

DYM "the" in place of "for"?

> attributes section, the current code in readelf used for dumping the object
> attributes incorrectly detects an overflow, and prints out an error message
> for a corrupted string tag.
> 
> This patch fixes the detection of the overflow so that it now accept an
> empty string in the last tag of the object attributes section.
> 
> Today, no test can be added since the bug cannot be triggered in the context
> of OAv1. This fix was tested in the context of OAv2's patch series [1] that
> will add tests to cover this specific use case.
> 
> [1]: https://inbox.sourceware.org/binutils/20250509151319.88725-1-matthieu
>      .longo@arm.com/
> ---
>  binutils/readelf.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index dd1871d8c75..b4efc02784a 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -17779,13 +17779,18 @@ display_tag_value (signed int tag,
>    else if (tag & 1)
>      {
>        /* PR 17531 file: 027-19978-0.004.  */
> -      size_t maxlen = (end - p) - 1;
> +      size_t maxlen = end - p;
>  
>        putchar ('"');
>        if (maxlen > 0)
>  	{
> -	  print_symbol_name ((int) maxlen, (const char *) p);
> -	  p += strnlen ((char *) p, maxlen) + 1;
> +	  maxlen -= 1; /* Remove \0 from the character count.  */
> +	  if (maxlen > 0) /* Don't try to print an empty string.  */

This line looks pointless, as ...

> +	    print_symbol_name ((int) maxlen, (const char *) p);

... the function will bail early if passed 0.

However - is using print_symbol_name() actually correct here? Surely it
is bogus to demangle this string, in the (however unlikely) event that it
turns out demanglable?

Jan