[PATCH] malloc: Print error when oldsize is not equal to the current size.

[PATCH] malloc: Print error when oldsize is not equal to the current size.

[liqingqing]

 This is used to detect errors early.

---
 malloc/malloc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 530c792997..243be0da68 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -4784,7 +4784,8 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
 
   /* oldmem size */
   if (__builtin_expect (chunksize_nomask (oldp) <= CHUNK_HDR_SZ, 0)
-      || __builtin_expect (oldsize >= av->system_mem, 0))
+      || __builtin_expect (oldsize >= av->system_mem, 0)
+      || __builtin_expect (oldsize != chunksize (oldp), 0))
     malloc_printerr ("realloc(): invalid old size");
 
   check_inuse_chunk (av, oldp);
-- 
2.27.0

Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.

[liqingqing]

ping again. 

the read of the oldsize is not protected by any lock, so check this value to avoid causing bigger mistakes. 

On 2021/3/26 19:05, liqingqing wrote:
>  This is used to detect errors early.
>
> ---
>  malloc/malloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 530c792997..243be0da68 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -4784,7 +4784,8 @@ _int_realloc(mstate av, mchunkptr oldp, INTERNAL_SIZE_T oldsize,
>  
>    /* oldmem size */
>    if (__builtin_expect (chunksize_nomask (oldp) <= CHUNK_HDR_SZ, 0)
> -      || __builtin_expect (oldsize >= av->system_mem, 0))
> +      || __builtin_expect (oldsize >= av->system_mem, 0)
> +      || __builtin_expect (oldsize != chunksize (oldp), 0))
>      malloc_printerr ("realloc(): invalid old size");
>  
>    check_inuse_chunk (av, oldp);

Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.

[DJ Delorie]

liqingqing <liqingqing3@huawei.com> writes:
> the read of the oldsize is not protected by any lock, so check this value to avoid causing bigger mistakes. 

Normally nothing can change oldsize until the oldp chunk is returned to
the arena, and at the point where you added the check that hasn't
happened.  Could you be more specific about how this value might change
out from under us?

Is this a case of "some other thread might corrupt this"?  But that can
happen regardless of lock.

Are you assuming some other malloc/free call could corrupt oldsize while
they hold the lock?  If so, is there a published exploit description for
this?

Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.

[liqingqing]

On 2021/4/14 6:14, DJ Delorie wrote:
> liqingqing <liqingqing3@huawei.com> writes:
>> the read of the oldsize is not protected by any lock, so check this value to avoid causing bigger mistakes. 
> Normally nothing can change oldsize until the oldp chunk is returned to
> the arena, and at the point where you added the check that hasn't
> happened.  Could you be more specific about how this value might change
> out from under us?
>
> Is this a case of "some other thread might corrupt this"?  But that can
> happen regardless of lock.
>
> Are you assuming some other malloc/free call could corrupt oldsize while
> they hold the lock?  If so, is there a published exploit description for
> this?
>
> .

>> hello DJ, thanks for your reply. there have two scenarios,

1, if two threads  call  realloc at the same time, and for the same  ptr.

   cause the oldsize is not protected by any lock, so if one thread has already finish resize the ptr and update the size,

  but the other thread still use the oldsize. so  after this thread got the mutex lock “__libc_lock_lock (ar_ptr->mutex)” in the _int_realloc,

  we should verify the  oldsize again.

2, two threads, one call free and the other call realloc,  with the same ptr.

    if the first " free" thread trigger memory consolidate and modify the ptr's size, 

   and then the other "realloc"  thread get the lock , but still use the oldsize, this'will cause corrupt.


although the man page has said this  will course unknow behavior, but i think this patch can reduce the impact.

Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.

[DJ Delorie]

LGTM; committed.  Sorry for the delay.

Reviewed-by: DJ Delorie <dj@redhat.com>