public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] malloc: Print error when oldsize is not equal to the current size.
@ 2021-03-26 11:05 liqingqing
  2021-04-01  8:51 ` liqingqing
  2022-09-22 19:51 ` DJ Delorie
  0 siblings, 2 replies; 5+ messages in thread
From: liqingqing @ 2021-03-26 11:05 UTC (permalink / raw)
  To: libc-alpha, carlos, dj, Siddhesh Poyarekar; +Cc: wuxu.wu, yebiaoxiang, Liusirui

 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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.
  2021-03-26 11:05 [PATCH] malloc: Print error when oldsize is not equal to the current size liqingqing
@ 2021-04-01  8:51 ` liqingqing
  2021-04-13 22:14   ` DJ Delorie
  2022-09-22 19:51 ` DJ Delorie
  1 sibling, 1 reply; 5+ messages in thread
From: liqingqing @ 2021-04-01  8:51 UTC (permalink / raw)
  To: libc-alpha, carlos, dj, Siddhesh Poyarekar; +Cc: wuxu.wu, yebiaoxiang, Liusirui

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);

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.
  2021-04-01  8:51 ` liqingqing
@ 2021-04-13 22:14   ` DJ Delorie
  2021-04-23  2:23     ` liqingqing
  0 siblings, 1 reply; 5+ messages in thread
From: DJ Delorie @ 2021-04-13 22:14 UTC (permalink / raw)
  To: liqingqing; +Cc: libc-alpha

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?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.
  2021-04-13 22:14   ` DJ Delorie
@ 2021-04-23  2:23     ` liqingqing
  0 siblings, 0 replies; 5+ messages in thread
From: liqingqing @ 2021-04-23  2:23 UTC (permalink / raw)
  To: DJ Delorie; +Cc: libc-alpha, wuxu.wu, Liusirui


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.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] malloc: Print error when oldsize is not equal to the current size.
  2021-03-26 11:05 [PATCH] malloc: Print error when oldsize is not equal to the current size liqingqing
  2021-04-01  8:51 ` liqingqing
@ 2022-09-22 19:51 ` DJ Delorie
  1 sibling, 0 replies; 5+ messages in thread
From: DJ Delorie @ 2022-09-22 19:51 UTC (permalink / raw)
  To: liqingqing; +Cc: libc-alpha


LGTM; committed.  Sorry for the delay.

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-09-22 19:52 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-26 11:05 [PATCH] malloc: Print error when oldsize is not equal to the current size liqingqing
2021-04-01  8:51 ` liqingqing
2021-04-13 22:14   ` DJ Delorie
2021-04-23  2:23     ` liqingqing
2022-09-22 19:51 ` DJ Delorie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).