[PATCH v12 06/25] readelf: dump Object Attributes v2
Jan Beulich
jbeulich@suse.com
Tue Jan 20 11:45:35 GMT 2026
On 20.01.2026 12:32, Matthieu Longo wrote:
> On 19/01/2026 12:02, Jan Beulich wrote:
>> On 16.01.2026 19:59, Matthieu Longo wrote:
>>> +static BufferReadOp_t
>>> +elf_parse_attrs_subsection_v2 (unsigned char *cursor,
>>
>> See https://sourceware.org/pipermail/binutils/2026-January/147519.html
>> wrt the lack of const-ness here.
>>
>>> + 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);
>>
>> Strictly speaking %u isn't right for uint32_t. Question is why uint32_t needs
>> using in the first place, when "unsigned int" would look to do.
>>
>
> <format-version: ‘A’>
> [ <uint32: subsection-length> NTBS: vendor-name
> <bytes: vendor-data>
> ]*
>
> subsection_len is defined as a uint32_t in the doc, hence my usage of it.
Well. If the layout was described in e.g. a struct to directly represent an
interface, surely uint32_t would need using. But the same isn't generally
true for local variables.
> Also, I discovered very recently that uint32_t could be different from unsigned int thanks to you in a previous revision.
> I have always used unsigned int as it always was 32 bits up until now, but I usually prefer to use uint32_t as it is more explicit.
> Anyway, I replaced uint32_t by unsigned int for this case.
Thanks.
> Here below is the proposed fix.
>
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -20098,6 +20098,17 @@ display_attr_v2 (const unsigned char *cursor,
> return oav2_display_attr_value (cursor, end, value_encoding);
> }
>
> +#define READ_SUBSEC_PROPERTY(var, read_len, start, end) \
> +do \
> + { \
> + if (start >= end) \
> + error \
> + (_("end of data encountered whilst reading property of subsection\n")); \
> + var = byte_get (start, read_len); \
> + start += read_len; \
> + } \
> +while (0)
> +
> typedef struct {
> bool err;
> uint64_t read;
> @@ -20129,9 +20140,9 @@ elf_parse_attrs_subsection_v2 (const unsigned char *cursor,
> return op;
> }
>
> - uint32_t subsection_len = byte_get (cursor, F_SUBSECTION_LEN);
> - op.read += F_SUBSECTION_LEN;
> + unsigned int subsection_len = byte_get (cursor, F_SUBSECTION_LEN);
> cursor += F_SUBSECTION_LEN;
> + op.read += F_SUBSECTION_LEN;
> if (subsection_len > max_read)
> {
> error (_("Bad subsection length: too big (%u > max=%lu)\n"),
> @@ -20194,12 +20205,12 @@ elf_parse_attrs_subsection_v2 (const unsigned char *cursor,
> printf (_(" Length: %u\n"), subsection_len);
>
> uint8_t optional;
> - READ_ULEB (optional, cursor, end);
> - op.read += 1;
> + READ_SUBSEC_PROPERTY (optional, sizeof (optional), cursor, end);
> + op.read += sizeof (optional);
>
> if (optional > 1)
> {
> - error (_("Optional value seems corrupted, got %u but only"
> + error (_("Optional value seems corrupted, got %d but only"
> " 0 (false) or 1 (true) are valid values\n"),
> optional);
> op.err = true;
> @@ -20210,14 +20221,16 @@ elf_parse_attrs_subsection_v2 (const unsigned char *cursor,
> printf (_(" Comprehension: %s\n"), optional ? "optional" : "required");
>
> uint8_t value_encoding_raw;
> - READ_ULEB (value_encoding_raw, cursor, end);
> - op.read += 1;
> + READ_SUBSEC_PROPERTY (value_encoding_raw, sizeof (value_encoding_raw),
> + cursor, end);
> + op.read += sizeof (value_encoding_raw);
> +
> enum obj_attr_encoding_v2 value_encoding
> = obj_attr_encoding_v2_from_u8 (value_encoding_raw);
>
> if (value_encoding > OA_ENC_MAX)
> {
> - error (_("Attribute type seems corrupted, got %u but only 0 (ULEB128)"
> + error (_("Attribute type seems corrupted, got %d but only 0 (ULEB128)"
> " or 1 (NTBS) are valid types\n"),
> value_encoding_raw);
> op.err = true;
Looks okay as an incremental change, but iirc there were more uint32_t that
my comment was touching (but then again it may also have been elsewhere; the
series is pretty long).
>>> + 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\n"),
>>> + sec_hdr->sh_size - remaining);
>>
>> Again %lx isn't valid to use with uint64_t.
>
> Used PRIu64 instead.
PRIx64 I hope?
Jan
More information about the Binutils
mailing list