Re: [PATCH] malloc/malloc.c: Mitigate null-byte overflow attacks

[Florian Weimer <fweimer at redhat dot com>]

On 01/15/2018 08:56 PM, Moritz Eckert wrote:
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index f5aafd2c05..d6ebfafd9a 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -4288,6 +4288,8 @@ _int_free (mstate av, mchunkptr p, int have_lock)
>         prevsize = prev_size (p);
>         size += prevsize;
>         p = chunk_at_offset(p, -((long) prevsize));
> +      if (__builtin_expect (chunksize(p) != prevsize, 0))
> +        malloc_printerr ("corrupted size vs. prev_size");
>         unlink(av, p, bck, fwd);
>       }
>   
> @@ -4449,6 +4451,8 @@ static void malloc_consolidate(mstate av)
>   	  prevsize = prev_size (p);
>   	  size += prevsize;
>   	  p = chunk_at_offset(p, -((long) prevsize));
> +	  if (__builtin_expect (chunksize(p) != prevsize, 0))
> +	    malloc_printerr ("corrupted size vs. prev_size");
>   	  unlink(av, p, bck, fwd);
>   	}

I think it would make sense to have different error messages in both 
cases, to make crash reports more meaningful.  And __glibc_unlikely 
should be used.

Otherwise, it looks good to me.  DJ, could you commit this in Moritz' 
name with a proper changelog entry?  It does not raise to the threshold 
of requiring copyright assignment.

Thanks,
Florian