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

[Matthieu Longo]

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.

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