[PATCH v0 06/15] readelf: add support to dump build attributes v2

[Matthieu Longo]

On 2025-03-11 08:12, Jan Beulich wrote:
> On 10.03.2025 18:51, Matthieu Longo wrote:
>> --- a/binutils/readelf.c
>> +++ b/binutils/readelf.c
>> @@ -58,6 +58,7 @@
>>   #define BFD64
>>   
>>   #include "bfd.h"
>> +#include "elf-attrs.h"
>>   #include "bucomm.h"
>>   #include "elfcomm.h"
>>   #include "demanguse.h"
>> @@ -19456,6 +19457,263 @@ free_data:
>>     return res;
>>   }
>>   
>> +static unsigned char *
>> +display_attr_v2 (unsigned char * cursor,
>> +		 const unsigned char * const end,
>> +		 const char *subsec_name,
>> +		 obj_attr_encoding_v2 value_encoding)
>> +{
>> +  static const known_tag_v2 known_tags_aeabi_feature_and_bits [] =
>> +  {
>> +    {Tag_Feature_BTI, "Feature_BTI", {0}},
>> +    {Tag_Feature_PAC, "Feature_PAC", {0}},
>> +    {Tag_Feature_GCS, "Feature_GCS", {0}},
>> +  };
>> +  static const known_tag_v2 known_tags_aeabi_pauthabi [] =
>> +  {
>> +    {Tag_PAuth_Platform, "PAuth_Platform", {0}},
>> +    {Tag_PAuth_Schema, "PAuth_Schema", {0}},
>> +  };
>> +  static const known_subsection_v2 known_subsections[] =
>> +  {
>> +    {
>> +      .subsec_name = "aeabi_feature_and_bits",
>> +      .known_tags = known_tags_aeabi_feature_and_bits,
>> +      .optional = true,
>> +      .encoding = ULEB128,
>> +      .len = sizeof (known_tags_aeabi_feature_and_bits),
>> +    },
>> +    {
>> +      .subsec_name = "aeabi_pauthabi",
>> +      .known_tags = known_tags_aeabi_pauthabi,
>> +      .optional = false,
>> +      .encoding = ULEB128,
>> +      .len = sizeof (known_tags_aeabi_pauthabi),
>> +    },
>> +  };
>> +
>> +  const known_subsection_v2 *
>> +  identify_subsection_ (const char* name)
>> +  {
>> +    for (unsigned i = 0; i < ARRAY_SIZE (known_subsections); ++i)
>> +      if (strcmp (name, known_subsections[i].subsec_name) == 0)
>> +	return &known_subsections[i];
>> +    return NULL;
>> +  }
>> +
>> +  const known_tag_v2 *
>> +  identify_tag_ (const known_subsection_v2* subsec,
>> +		 uint32_t tag)
>> +  {
>> +    for (unsigned i = 0; i < subsec->len; ++i)
>> +      {
>> +	const known_tag_v2 *known_tag = &subsec->known_tags[i];
>> +	if (known_tag->tag == tag)
>> +	  return known_tag;
>> +      }
>> +    return NULL;
>> +  }
>> +
>> +  uint32_t tag;
>> +  READ_ULEB (tag, cursor, end);
>> +
>> +  const known_tag_v2 *tag_info = NULL;
>> +  const known_subsection_v2 *subsec_info = identify_subsection_ (subsec_name);
>> +  if (subsec_info != NULL)
>> +    tag_info = identify_tag_ (subsec_info, tag);
>> +
>> +  if (tag_info != NULL)
>> +    printf ("    Tag_%s:	", tag_info->name);
>> +  else
>> +    printf ("    Tag_unknown_%u:	", tag);
>> +
>> +  switch (value_encoding)
>> +    {
>> +    case NTBS:
>> +      cursor = display_tag_value (-1, cursor, end);
>> +      break;
>> +    case ULEB128:
>> +      cursor = display_tag_value (0, cursor, end);
>> +      break;
>> +    }
>> +
>> +  return cursor;
>> +}
>> +
>> +typedef struct {
>> +  bool err;
>> +  uint64_t read;
>> +} BufferReadOp_t ;
>> +
>> +static BufferReadOp_t
>> +elf_parse_attrs_subsection_v2 (unsigned char* cursor,
>> +			       const uint64_t max_read,
>> +			       const char* public_name)
>> +{
>> +  BufferReadOp_t op = { .err = false, .read = 0 };
>> +
>> +  const uint32_t F_SUBSECTION_LEN = sizeof(uint32_t);
>> +  /* The minimum subsection length is 5: 4 bytes for the length itself, and 1
>> +     byte for an empty NUL-terminated string, and no vendor-data.  */
>> +  const uint32_t F_MIN_SUBSECTION_DATA_LEN = F_SUBSECTION_LEN + 1;
>> +
>> +  if (max_read <= F_SUBSECTION_LEN)
>> +    {
>> +      error (_("Build attributes section ends prematurely\n"));
>> +      return op;
>> +    }
>> +  uint32_t subsection_len = byte_get (cursor, F_SUBSECTION_LEN);
>> +  op.read += F_SUBSECTION_LEN;
>> +  cursor += F_SUBSECTION_LEN;
>> +  if (subsection_len > max_read)
>> +    {
>> +      error (_("Bad subsection length (%u > max=%lu)\n"),
>> +	     subsection_len, max_read);
>> +      // error, but still try to display the content until meeting a more serious error.
>> +      subsection_len = max_read;
>> +      op.err = true;
>> +    }
>> +  /* PR 17531: file: 001-101425-0.004  */
> 
> A comment like this, referencing a really old PR, suggests code is being copied
> when - if at all possible - code would better be re-used (with refactoring as
> necessary).
> 

Richard Ball, the initial author of this patch, tried to refactor the 
code of Object Attributes v1 but it was making things more difficult to 
understand, and introduced a risk of regressions.

This code was not copied but the reference to PR17531 was added because 
it would be basically the same issue, i.e. the length of the subsection 
that is specified is greater than the available size of remaining data 
in the section.

>> +  else if (subsection_len < F_MIN_SUBSECTION_DATA_LEN)
>> +    {
>> +      error (_("Subsection length of %u is too small\n"), subsection_len);
>> +      op.err = true;
>> +      return op;
>> +    }
>> +
>> +  size_t subsection_name_len = strnlen ((char *) cursor, subsection_len) + 1;
>> +  if (subsection_name_len >= subsection_len)
>> +    {
>> +      error (_("Subsection name seems corrupted (missing '\\0')\n"));
>> +      op.err = true;
>> +      return op;
>> +    }
>> +  //if (subsection_name_len == 0)
>> +  //  // do something here when the string is '\0'
> 
> What's this? Was this meant to be removed before submitting?
> 

Yes, I asked for clarification with the spec. Such a name is valid but 
useless. I replaced this commented code with a detailed comment.

> Jan