gazillon of double-free

gazillon of double-free

[Robert Millan]

It seems we have a ton of double-free bugs in label() and
uuid() routines.

Take for example grub_ext2_label():

  data = grub_ext2_mount (disk);
  if (data)
    *label = grub_strndup (data->sblock.volume_name, 14);
  else
    *label = NULL;
  grub_free (data);

If grub_ext2_mount fails, data is not allocated but we free it anyway.

Or perhaps I'm missing something? (it's late here, I need some sleep)

-- 
Robert Millan

Re: gazillon of double-free

[Vladimir 'φ-coder/phcoder' Serbinenko]

On 09/09/10 01:20, Robert Millan wrote:
> It seems we have a ton of double-free bugs in label() and
> uuid() routines.
>
> Take for example grub_ext2_label():
>
>    data = grub_ext2_mount (disk);
>    if (data)
>      *label = grub_strndup (data->sblock.volume_name, 14);
>    else
>      *label = NULL;
>    grub_free (data);
>
> If grub_ext2_mount fails, data is not allocated but we free it anyway.
>
> Or perhaps I'm missing something? (it's late here, I need some sleep)
>
>    
grub_free (NULL) is a no-op on purpose:
/* Deallocate the pointer PTR.  */
void
grub_free (void *ptr)
{
   grub_mm_header_t p;
   grub_mm_region_t r;

   if (! ptr)
     return;



-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko

Re: gazillon of double-free

[Colin D Bennett]

On Thu, 9 Sep 2010 01:20:40 +0200
Robert Millan <rmh@gnu.org> wrote:

> It seems we have a ton of double-free bugs in label() and
> uuid() routines.
> 
> Take for example grub_ext2_label():
> 
>   data = grub_ext2_mount (disk);
>   if (data)
>     *label = grub_strndup (data->sblock.volume_name, 14);
>   else
>     *label = NULL;
>   grub_free (data);

It is important to distinguish between double-free and free(NULL).
As Vladimir points out, free(NULL) is of course OK, but 
"free(p); free(p);",  where p != NULL, is a double-free and is wrong.

Regards,
Colin