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

[Matthieu Longo]

On 2025-06-18 12:07, Matthieu Longo wrote:
> On 2025-06-18 11:42, Jan Beulich wrote:
>> On 18.06.2025 12:27, 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.
>>>
>>> In the context of OAv2 [1], an empty string is a valid value for a 
>>> string
>>> attribute tag, and can be saved as a tag might not have a default value.
>>
>> Is there a word (or more) missing in the latter part of this sentence?
> 
> All the words are here, but my phrasing is probably poor.
> 
> What I meant is that:
> 1. an empty string is a valid value for a tag for both OAv1 and OAv2.
> 2. contrarily to OAv1, with OAv2, subsections can be required and so 
> tags might need to be present even if the value is an empty string.
> 3. the OAv2 serializer won't drop the default values.
> 
> What about phrasing it like the below. Is it better ?
> 
> 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.
> 
>>> --- a/binutils/readelf.c
>>> +++ b/binutils/readelf.c
>>> @@ -17779,12 +17779,14 @@ 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);
>>> +      maxlen -= 1; /* Remove \0 from the character count.  */
>>
>> How do you know it's \0 that sits there?
> 
> "else if (tag & 1)" checks whether it is a NTBS or a ULEB128.
> So anything in this block assumes that the value is an NTBS.
> If the value does not end with '\0', it means that it is a corrupt 
> string. In this case, the last character won't be printed.
> 
> I think it might make sense to check whether the last character is `\0` 
> or not, and if it is not, then we fall back to printing out the error 
> message.
> 
> I will address the issue in the next revision.
> 

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.  */
+           print_symbol_name ((int) maxlen, (const char *) p);
+         size_t len = strnlen ((char *) p, maxlen);
+         if (len == maxlen && p[maxlen] != '\0')
+           printf (_("<corrupt string tag>"));
+         p += len + 1;
         }
        else
         {

>>> +      if (maxlen > 0) /* Don't try to print an empty string.  */
>>> +        print_symbol_name ((int) maxlen, (const char *) p);
>>
>> Am I understanding correctly that for OAv2 this will then need further
>> modifying, as (if I understand the description correctly) an empty
>> string there is not the same as no tag at all.
> 
> No, it won't need further modification.
> 
>>
>> Jan
> 
> Matthieu.