Re: [PATCH v2 2/7] malloc: Additional checks for unsorted bin integrity I.

[Florian Weimer <fweimer at redhat dot com>]

On 11/07/2017 04:27 PM, Istvan Kurucsai wrote:
> +          next = chunk_at_offset (victim, size);

For new code, we prefer declarations with initializers.

> +          if (__glibc_unlikely (chunksize_nomask (victim) <= 2 * SIZE_SZ)
> +              || __glibc_unlikely (chunksize_nomask (victim) > av->system_mem))
> +            malloc_printerr("malloc(): invalid size (unsorted)");
> +          if (__glibc_unlikely (chunksize_nomask (next) < 2 * SIZE_SZ)
> +              || __glibc_unlikely (chunksize_nomask (next) > av->system_mem))
> +            malloc_printerr("malloc(): invalid next size (unsorted)");
> +          if (__glibc_unlikely ((prev_size (next) & ~(SIZE_BITS)) != size))
> +            malloc_printerr("malloc(): mismatching next->prev_size (unsorted)");

I think this check is redundant because prev_size (next) and chunksize 
(victim) are loaded from the same memory location.

> +          if (__glibc_unlikely (bck->fd != victim)
> +              || __glibc_unlikely (victim->fd != unsorted_chunks (av)))
> +            malloc_printerr("malloc(): unsorted double linked list corrupted");
> +          if (__glibc_unlikely (prev_inuse(next)))
> +            malloc_printerr("malloc(): invalid next->prev_inuse (unsorted)");

There's a missing space after malloc_printerr.

Why do you keep using chunksize_nomask?  We never investigated why the 
original code uses it.  It may have been an accident.

Again, for non-main arenas, the checks against av->system_mem could be 
made tighter (against the heap size).  Maybe you could put the condition 
into a separate inline function?

Thanks,
Florian