[Bug libc/25912] New: Possible race in free_check() with MALLOC_CHECK_=3

[Bug libc/25912] New: Possible race in free_check() with MALLOC_CHECK_=3

[vvijayan at mathworks dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25912

            Bug ID: 25912
           Summary: Possible race in free_check() with MALLOC_CHECK_=3
           Product: glibc
           Version: 2.28
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libc
          Assignee: unassigned at sourceware dot org
          Reporter: vvijayan at mathworks dot com
                CC: drepper.fsp at gmail dot com
  Target Milestone: ---

There seems to be a race in free_check() with MALLOC_CHECK_=3 is set, I guess 
after the following change
https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=ac3ed168d0c0b2b702319ac0db72c9b475a8c72e, 
 static void *
@@ -308,13 +273,7 @@ free_check (void *mem, const void *caller)
   __libc_lock_lock (main_arena.mutex);
   p = mem2chunk_check (mem, NULL);
   if (!p)
-    {
-      __libc_lock_unlock (main_arena.mutex);
-
-      malloc_printerr (check_action, "free(): invalid pointer", mem,
-                      &main_arena);
-      return;
-    }
+    malloc_printerr ("free(): invalid pointer");
   if (chunk_is_mmapped (p))
     {
       __libc_lock_unlock (main_arena.mutex);

The above code leaves the main_arena.mutex locked.
In case if from signal handler a new malloc request is raised, the thread seems
to be hanging forever.
The following is the just an example to reproduce this.

#include <stdio.h>
#include<unistd.h>
#include<signal.h>
#include <stdlib.h>

void abort_handler(int signo)
{
    char *p2 = (char*)malloc(sizeof(char) * 32);
     p2 = "*******recieved SIGABRT******\n";

      if (signo == SIGABRT)
          printf("%s",p2);
      //free(p2);
      exit(1);
}

int main() {
    char *str = (char*) malloc(16);
    (void)signal(SIGABRT, abort_handler);
    str = str + 0x10;
    free(str);

    return 0;
}

$gcc test.c -g
$ 
/* Works fine with MALLOC_CHECK_ unset
$./a.out 
free(): invalid pointer
*******recieved SIGABRT******
$

$ export MALLOC_CHECK_=3
/************ Program hangs ***************/
$./a.out 
free(): invalid pointer

^C
$

Backtrace in gdb
(gdb) 
8           char *p2 = (char*)malloc(sizeof(char) * 32);
(gdb) 
^C
Program received signal SIGINT, Interrupt.
__lll_lock_wait_private () at
../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:63
63      ../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: No such file or
directory.
(gdb) bt
#0  __lll_lock_wait_private () at
../sysdeps/unix/sysv/linux/x86_64/lowlevellock.S:63
#1  0x00007ffff7e60516 in malloc_check (sz=140737353714752, caller=<optimized
out>) at hooks.c:236
#2  0x0000000000400637 in abort_handler (signo=6) at test3.c:8
#3  <signal handler called>
#4  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#5  0x00007ffff7dff535 in __GI_abort () at abort.c:79
#6  0x00007ffff7e56508 in __libc_message (action=action@entry=do_abort,
fmt=fmt@entry=0x7ffff7f6128d "%s\n") at ../sysdeps/posix/libc_fatal.c:181
#7  0x00007ffff7e5cc1a in malloc_printerr (str=str@entry=0x7ffff7f5f43b
"free(): invalid pointer") at malloc.c:5341
#8  0x00007ffff7e60b3e in free_check (mem=<optimized out>, caller=<optimized
out>) at hooks.c:254
#9  0x000000000040069f in main () at test3.c:21
(gdb) p main_arena.mutex
$1 = 2
(gdb) p main_arena
$2 = {mutex = 2, flags = 0, have_fastchunks = 0, fastbinsY = {0x0, 0x0, 0x0,
0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, top = 0x602020, last_remainder = 0x0,


Could you please confirm if this is a bug?

Regards,
Vinitha

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25912] Possible race in free_check() with MALLOC_CHECK_=3

[vvijayan at mathworks dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25912

Vinitha Vijayan <vvijayan at mathworks dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |fweimer at redhat dot com

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug libc/25912] Possible race in free_check() with MALLOC_CHECK_=3

[carlos at redhat dot com]

https://sourceware.org/bugzilla/show_bug.cgi?id=25912

Carlos O'Donell <carlos at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID
                 CC|                            |carlos at redhat dot com

--- Comment #1 from Carlos O'Donell <carlos at redhat dot com> ---
(In reply to Vinitha Vijayan from comment #0)
> In case if from signal handler a new malloc request is raised, the thread
> seems to be hanging forever.

Your test is invalid.

Please see:
https://www.gnu.org/software/libc/manual/html_node/Basic-Allocation.html#Basic-Allocation

malloc() is marked AS-unsafe, which means it is not safe to call in an
asynchronous signal handler.

Likewise review 'man signal-safety', and see that 'malloc' is not in the list
of signal-safe functions.

> Could you please confirm if this is a bug?

This is not a bug. The program has detected the invalid free and raised a
SIGABRT, but your handler does something invalid and you hang.

I'm marking this RESOLVED/INVALID.

-- 
You are receiving this mail because:
You are on the CC list for the bug.