Possible Memory leak in bed hash.c

Possible Memory leak in bed hash.c

[jacob navia]

[-- Attachment #1: Type: text/plain, Size: 1632 bytes --]

Function: bfd_elf_strtab_init, file hash.c lines 94-126

Type of bug: Memory leak
-------------------

Description:
-------------

/* Create a new hash table.  */
struct elf_strtab_hash *_bfd_elf_strtab_init(void)
{
    struct elf_strtab_hash *table;
    size_t      amt = sizeof(struct elf_strtab_hash);

    table = (struct elf_strtab_hash *)malloc(amt);
    if (table == NULL) 
        return NULL; 
    // This call allocates several fields in the table.
    if (!bfd_hash_table_init(&table->table,elf_strtab_hash_newfunc,
                 sizeof(struct elf_strtab_hash_entry))) {
        free(table);
        return NULL; 
    }
    table->sec_size = 0;
    table->size = 1;
    table->alloced = 64; 
    amt = sizeof(struct elf_strtab_hasn_entry *);
    table->array = ((struct elf_strtab_hash_entry **)
            malloc(table->alloced * amt));
    if (table->array == NULL) {
        free(table);          <<<<<<<<<<<<<<<< MEMORY LEAK                                                                                             
        return NULL; 
    }
    table->array[0] = NULL; 

    return table;
}

We call « bfd_hash_table_init" that initializes the table with several huge structures. It returns OK, and we go on with table->sec_size = 0; etc.

Then, we attempt to allocate the array.

If it fails, we free just the table, leaking all previously allocated subfields.

HOW TO FIX:
—————

Just call « bfd_hash_table_free » instead of « free » 

Priority: LOW
In these times of plenty (gigabytes of RAM, etc) nobody cares about writing good software. 

Re: Possible Memory leak in bed hash.c

[Alan Modra]

On Tue, Sep 12, 2023 at 02:05:29PM +0200, jacob navia wrote:
> Just call « bfd_hash_table_free » instead of « free » 

Not quite.  We should add a call

            * elf-strtab.c (_bfd_elf_strtab_init): Call bfd_hash_table_free
            on error return path.

diff --git a/bfd/elf-strtab.c b/bfd/elf-strtab.c
index 5de5af73bb4..d52a3079f4c 100644
--- a/bfd/elf-strtab.c
+++ b/bfd/elf-strtab.c
@@ -116,6 +116,7 @@ _bfd_elf_strtab_init (void)
 		  bfd_malloc (table->alloced * amt));
   if (table->array == NULL)
     {
+      bfd_hash_table_free (&table->table);
       free (table);
       return NULL;
     }

-- 
Alan Modra
Australia Development Lab, IBM

Re: Possible Memory leak in bed hash.c

[Nick Clifton]

On 9/13/23 07:37, Alan Modra via Binutils wrote:
> Not quite.  We should add a call
> 
>              * elf-strtab.c (_bfd_elf_strtab_init): Call bfd_hash_table_free
>              on error return path.

Patch applied.

Cheers
   Nick

Re: Possible Memory leak in bed hash.c

[Nick Clifton]

Hi Jacob,

> Priority: LOW
> In these times of plenty (gigabytes of RAM, etc) nobody cares about writing good software.
This is just a quick note to say that we do care.

We care about writing good code.  We care about writing portable code.
We care about writing code that others will see, read and use, and we
hope, appreciate.

So please, do keep on filing bug reports and letting us know about problems,
but do not think that we do not care.  We do.

Cheers
   Nick

PS.  Mind you the thing I do not care about are when I get a CVE report
for one of these memory leak bugs saying that the leak is a potential
source of a denial of service attack.  I mean seriously, if one of the
binutils tools does not free all of its memory before exiting, can that
really be used as a DoS ?