[PATCH v11 06/25] readelf: dump Object Attributes v2

[Matthieu Longo]

On 08/01/2026 13:36, Jan Beulich wrote:
> On 31.12.2025 00:05, Matthieu Longo wrote:
>> +static BufferReadOp_t
>> +elf_parse_attrs_subsection_v2 (unsigned char *cursor,
>> +			       const uint64_t max_read,
>> +			       const char *public_name,
>> +			       display_arch_attr_t display_arch_attr)
>> +{
>> +  BufferReadOp_t op = { .err = false, .read = 0 };
>> +
>> +  const uint32_t F_SUBSECTION_LEN = sizeof (uint32_t);
>> +  const uint32_t F_SUBSECTION_COMPREHENSION = sizeof(uint8_t);
>> +  const uint32_t F_SUBSECTION_ENCODING = sizeof(uint8_t);
>> +  /* The minimum subsection length is 7: 4 bytes for the length itself, and 1
>> +     byte for an empty NUL-terminated string, 1 byte for the comprehension,
>> +     1 byte for the encoding, and no vendor-data.  */
>> +  const uint32_t F_MIN_SUBSECTION_DATA_LEN
>> +    = F_SUBSECTION_LEN + 1 /* for '\0' */
>> +      + F_SUBSECTION_COMPREHENSION + F_SUBSECTION_ENCODING;
>> +
>> +  /* Handle cases where the attributes data is not strictly valid (e.g. due to
>> +     fuzzing).  */
>> +  if (max_read < F_MIN_SUBSECTION_DATA_LEN)
>> +    {
>> +      error (_("Object 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: too big (%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;
>> +    }
>> +  else if (subsection_len < F_MIN_SUBSECTION_DATA_LEN)
>> +    {
>> +      error (_("Bad subsection length: too small (%u < min=%u)\n"),
>> +	     subsection_len, F_MIN_SUBSECTION_DATA_LEN);
>> +      /* Error, but still try to display the content until meeting a more
>> +	 serious error.  */
>> +      subsection_len = max_read;
>> +      op.err = true;
>> +    }
>> +
>> +  const size_t MAX_SUBSECTION_NAME_LEN
>> +    = subsection_len - F_SUBSECTION_LEN
>> +      - F_SUBSECTION_COMPREHENSION - F_SUBSECTION_ENCODING;
>> +  size_t subsection_name_len
>> +    = strnlen ((char *) cursor, MAX_SUBSECTION_NAME_LEN);
>> +  if (subsection_name_len >= MAX_SUBSECTION_NAME_LEN)
>> +    {
>> +      error (_("Subsection name seems corrupted (missing '\\0')\n"));
>> +      op.err = true;
>> +      return op;
>> +    }
>> +  /* Note: if the length of the subsection name is 0 (i.e. the string is '\0'),
>> +     it is still considered a valid name for dumping, and an empty string will
>> +     be displayed.
>> +     However, in practice, such a name would be unexploitable by the linker
>> +     during the merge, thus the subsection would be dropped.  */
>> +  subsection_name_len += 1;
>> +
>> +  /* Note: at this stage,
>> +     1. the length of the subsection name is validated, as the presence of '\0'
>> +	at the end of the string, so no risk of buffer overrun.
>> +     2. the data for comprehension and encoding can also safely be read.  */
>> +  unsigned char *const end = cursor + subsection_len - F_SUBSECTION_LEN;
>> +  while (cursor < end)
>> +    {
>> +      const char *subsec_name = (const char *) cursor;
>> +      printf (_(" - Name:	  %s\n"), subsec_name);
>> +      /* The code below needs to be kept in sync with the code of
>> +	 bfd_elf_obj_attr_subsection_v2_scope() in bfd/elf-attrs.c.  */
>> +      size_t public_name_len = strlen (public_name);
>> +      bool public_subsection
>> +	= strncmp (subsec_name, public_name, public_name_len) == 0
>> +	  && subsec_name[public_name_len] == '_';
>> +      cursor += subsection_name_len;
>> +      op.read += subsection_name_len;
>> +
>> +      printf (_("   Scope:	  %s\n"),
> 
> If Scope is intended to be translated, ...
> 
>> +	      public_subsection ? "public" : "private");
> 
> ... shouldn't these two strings be, too?
> 

I discussed this before with Richard Earnshaw.
We concluded that, because those words are "keywords" coming from the doc, they should not be translated.

>> +      printf (_("   Length:	  %u\n"), subsection_len);
>> +
>> +      uint8_t optional;
>> +      READ_ULEB (optional, cursor, end);
>> +      op.read += 1;
>> +
>> +      if (optional > 1)
>> +	{
>> +	  error (_("Optional value seems corrupted, got %u but only"
>> +		   " 0 (false) or 1 (true) are valid values."),
> 
> There's \n missing here, and generally error messages shouldn't have a full
> stop at the end. (Looks to be an issue elsewhere as well.)
> 

Fixed here, and others places too.

>> +static bool
>> +process_attributes_v2 (Filedata *filedata,
>> +		       const char *public_name,
>> +		       uint32_t section_type,
>> +		       display_arch_attr_t display_arch_attr)
>> +{
>> +  /* Find the section header so that we get the size.  */
>> +  Elf_Internal_Shdr *sec_hdr = find_section_by_type (filedata, section_type);
>> +  if (sec_hdr == NULL)
>> +    /* No section, exit without error.  */
>> +    return true;
>> +
>> +  unsigned char *const data = (unsigned char *)
>> +    get_data (NULL, filedata, sec_hdr->sh_offset, 1, sec_hdr->sh_size,
>> +	      _("object attributes"));
>> +  if (data == NULL)
>> +    return false;
>> +
>> +  unsigned char *cursor = data;
>> +  bool res = true;
>> +
>> +  /* The first character is the version of the attributes.
>> +     Currently only version 1, (aka 'A') is recognised here.  */
>> +  if (*cursor != 'A')
>> +    {
>> +      error (_("Unknown attributes version '%c'(0x%02x) - expecting 'A'\n"),
>> +	     ISPRINT(*cursor) ? *cursor : '?', *cursor);
>> +      res = false;
>> +      goto free_data;
>> +    }
>> +
>> +  ++cursor;
>> +
>> +  printf (_("Subsections:\n"));
>> +  BufferReadOp_t op;
>> +  for (uint64_t remaining = sec_hdr->sh_size - 1; // already read 'A'
>> +       remaining > 1;
>> +       remaining -= op.read, cursor += op.read)
>> +    {
>> +      op = elf_parse_attrs_subsection_v2 (cursor, remaining, public_name,
>> +					  display_arch_attr);
>> +      if (op.err)
>> +	{
>> +	  error (_("Cannot parse subsection at offset %lx"),
>> +	    sec_hdr->sh_size - remaining);

Fixed missing \n.

>> +	  res = false;
>> +	  goto free_data;
>> +	}
>> +    }
>> +
>> +free_data:
> 
> At least one blank please ahead of labels.
> 

Fixed.

> Okay with these cosmetics taken care of.
> 
> Jan

Matthieu