[PATCH v1 4/7] bfd: fix memory leak when default-initializing an OAv2 attribute

Matthieu Longo matthieu.longo@arm.com
Thu Feb 5 15:18:51 GMT 2026


On 05/02/2026 10:52, Jan Beulich wrote:
> On 03.02.2026 11:00, Matthieu Longo wrote:
>> --- a/bfd/elf-attrs.c
>> +++ b/bfd/elf-attrs.c
>> @@ -1078,7 +1078,10 @@ oav2_attr_overwrite_with_default (const struct bfd_link_info *info,
>>         if (subsec->encoding == OA_ENC_ULEB128)
>>   	attr->val.uint = 0;
>>         else
>> -	attr->val.string = NULL;
>> +	{
>> +	  free ((char *) attr->val.string);
>> +	  attr->val.string = NULL;
>> +	}
>>         return;
>>       }
> 
> This is okay as is, yet I'd still like to ask: Much like you centralized things
> in patch 3, how about doing so here as well? Possibly even the function introduced
> there could be re-used here, maybe with the "merge" in its name dropped.
> 

Fixed.

>> @@ -1089,7 +1092,7 @@ oav2_attr_overwrite_with_default (const struct bfd_link_info *info,
>>       {
>>         if (attr->val.string != NULL)
>>   	{
>> -	  free ((void *) attr->val.string);
>> +	  free ((char *) attr->val.string);
>>   	  attr->val.string = NULL;
>>   	}
> 
> Why this change? void * _is_ the type free() takes. If you want both sides to be
> consistent, I'd rather see you use void * in the earlier hunk as well.

Fixed.

> 
> Jan

New version using oav2_assign_value().

Matthieu


diff --git a/bfd/elf-attrs.c b/bfd/elf-attrs.c
index e0847a04aec..c4f81ba5f6b 100644
--- a/bfd/elf-attrs.c
+++ b/bfd/elf-attrs.c
@@ -1083,33 +1083,30 @@ oav2_attr_overwrite_with_default (const struct bfd_link_info *info,
  {
    const struct elf_backend_data *bed = get_elf_backend_data (info->output_bfd);

+  union obj_attr_value_v2 default_value;
+  memset (&default_value, 0, sizeof (default_value));
+
    const obj_attr_info_t *attr_info
      = _bfd_obj_attr_v2_find_known_by_tag (bed, subsec->name, attr->tag);
    if (attr_info == NULL)
      {
        attr->status = obj_attr_v2_unknown;
-      if (subsec->encoding == OA_ENC_ULEB128)
-       attr->val.uint = 0;
-      else
-       attr->val.string = NULL;
+      oav2_assign_value (subsec->encoding, attr, default_value);
        return;
      }

    if (bed->obj_attr_v2_default_value != NULL
        && bed->obj_attr_v2_default_value (info, attr_info, subsec, attr))
-    {}
-  else if (subsec->encoding == OA_ENC_NTBS)
+    return;
+
+  if (subsec->encoding == OA_ENC_NTBS)
      {
-      if (attr->val.string != NULL)
-       {
-         free ((void *) attr->val.string);
-         attr->val.string = NULL;
-       }
        if (attr_info->default_value.string != NULL)
-       attr->val.string = xstrdup (attr_info->default_value.string);
+       default_value.string = xstrdup (attr_info->default_value.string);
      }
    else
-    attr->val.uint = attr_info->default_value.uint;
+    default_value.uint = attr_info->default_value.uint;
+  oav2_assign_value (subsec->encoding, attr, default_value);
  }

  /* Create a new attribute with the same key (=tag) as ATTR, and initialized with


More information about the Binutils mailing list