Linux C "free(): double free detected in tcache2

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Servus,

es klemmt mal wieder.

Ich bekomme die Fehlermeldung nach fork() und Ausführen des Threads
unmittelbar bei oder vor exit( 0);. Alle free() in allen Dateien sind
durch eine Abfrage ob der pointer != NULL ist abgesichert.

Im Web gibt es einige Seiten die auf ein Problem mit dem Thread-Cache
hinweisen - so ganz blicke ich da aber nicht durch (vor Allem was ich
da machen kann).

Hat vielleicht jemand eine Idee ?

Ubuntu, GCC

[MaWin (Gast)]

Joachim D. schrieb:
> Alle free() in allen Dateien sind
> durch eine Abfrage ob der pointer != NULL ist abgesichert.

unnötig.

> free(): double free detected

Das bedeutet, dass du free() zwei mal für den selben Pointer ausrufst.
Das kann auch jeweils einmal aus zwei Threads passieren.

Das ist ein schwerer Fehler und führt zu Undefined Behavior.

[Oliver S. (oliverso)]

Joachim D. schrieb:
> Alle free() in allen Dateien sind
> durch eine Abfrage ob der pointer != NULL ist abgesichert.

Gegen was genau?

Ansonsten wird das Problem genau das sein, was die Fehlermeldung besagt.

Oliver

[foobar (Gast)]

fork erzeugt einen neuen Prozess, keinen Thread - gemeinsam genutzter 
Speicher entfällt also.  exit in Kindprozessen ist meist unpassend - 
üblich ist _exit.  Der überflüssige NULL-check vor free ist bereits 
genannt worden.  free wird auch innerhalb der Libraries genutzt - ein 
doppelter fclose z.B. erzeugt auch so eine Meldung.

[Irgend W. (Firma: egal) (irgendwer)]

Joachim D. schrieb:
> Alle free() in allen Dateien sind
> durch eine Abfrage ob der pointer != NULL ist abgesichert
Das funktioniert aber nur wenn du den Pointer auch irgendwann auf Null 
setzt. Dein free macht das nicht von alleine.

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Die free()'s habe ich pingelig kontrolliert, da ist eigentlich
nichts. Ein free() auf einen schon freigebenen ptr läßt den
Prozess eigentlich nicht crashen.

Was bedeutet das tcache2 ?

Die Meldung kommt unmittelbar am Programmende, vor oder mit
exit() oder _exit().

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Am Prozessende ;)

[Norbert (Gast)]

Ich kann deinen Sourcecode von hier aus nicht erkennen,
aber mein erster Gedanke:

malloc() vor dem fork()?
free() in den beiden Prozessen?

Dat geit net.

fork() klont dir zwar die Pointer, aber nicht den allozierten Speicher.

[MaWin (Gast)]

Joachim D. schrieb:
> Ein free() auf einen schon freigebenen ptr läßt den
> Prozess eigentlich nicht crashen.

Eben doch. Das ist ein Double-Free. Ein schwerer Speicherfehler. Das ist 
Undefined Behavior.

> Die free()'s habe ich pingelig kontrolliert, da ist eigentlich
> nichts.

Dann kontrolliere es noch einmal.

Oder entferne einfach mal alle free()-Aufrufe und lecke den Speicher.
Tritt es immer noch auf? Nein -> Dann wirds wohl einer deiner 
free()-Aufrufe gewesen sein. Ja -> Dann wirds wohl irgendeine andere 
Ressource über Bande sein, die du doppelt freigibst.

[MaWin (Gast)]

Norbert schrieb:
> fork() klont dir zwar die Pointer, aber nicht den allozierten Speicher.

Ehm, doch. Natürlich tut es das. (Genauer gesagt legt es eine COW-Kopie 
an).

[Joachim D. (Firma: JDCC) (scheppertreiber)]

        while( 1){
                size = sizeof( a1);
                fd = accept( s, (struct sockaddr*) &a1, &size);

                if ( ( pid = fork()) < 0) {
                        printf( "\nfork err %s", strerror( errno));
                } else {
                        if ( pid == 0) {
                                close( s);
                                DoCmd( fd);
                        } else {
                                close( fd);
                        }
                }
        }

       static  int
        DoCmd(
        int fd
        )
{
        int     i, st, size, pid;
        char    http_header[4096];
        char    *p, *q, ip[256], req[1044], str[2048];
        char    *tmp = NULL;

        JA( "****************************** INCOMING DATA %s / %s ************************************", sysdatel(), systime());

        jStartTime();
        LoadServicesConf();

        strcpy( ip, inet_ntoa( a1.sin_addr));

        // vorher mal Speicher resrvieren, bei GET wird die Länge nicht
        // übergeben. 16kB reichen für GET.

        if ( (tmp = calloc( 1, 16384)) == NULL)
                ExitError( "GetRecData", "A alloc err tmp");

        size = GetRecData( fd, &tmp, ip);
        memset( http_header, 0, sizeof( http_header));
        mopen( tmp, strlen( tmp));
        st = 0;
        while ( mgetstrim( str)){
                if ( ! *str)
                        break;
                appsprintf( http_header, "%s\n", str);
                st += strlen( str);
        }

        printf( "\n  %s %-16s %8d Byte ", systime(), ip, size);
        fflush( stdout);

        // Kram ist da, also ausführen

        if ( (sfp = fopen( tmpfilename( ip), "w")) == NULL){
                JA( "open error %s %s", tmpfilename( ip), strerror( errno));
                exit( 0);
        }

        GetInputData( fd, tmp, size, ip, http_header);

        fclose( sfp);

        if ( tmp != NULL)
                free( tmp);

        // Senden des Ergebnisses

//        SendFileToHTTP( fd, ip);
        close( fd);
        _exit( 0);
}

(Auszug) tmp bekommt in GetRecData() evtl. per realloc()
mehr Platz wenn's ein POST ist.

[Joachim D. (Firma: JDCC) (scheppertreiber)]

MaWin schrieb:
> Oder entferne einfach mal alle free()-Aufrufe und lecke den Speicher.
> Tritt es immer noch auf? Nein -> Dann wirds wohl einer deiner
> free()-Aufrufe gewesen sein. Ja -> Dann wirds wohl irgendeine andere
> Ressource über Bande sein, die du doppelt freigibst.

Habe ich schon probiert (wenn der Prozess endelt sollte er ja
eh alles freigeben). Vielleicht ein überzähliges close() ?
Was ist dieser tcache 2 ?

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Auf stackoverflow gefunden:



The Thread Local Cache (tcache) is a performance optimization in glibc. 
Unfortunately, it comes at the expense of security and make some attacks 
much easier, as you have since discovered.

From 
https://sourceware.org/glibc/wiki/MallocInternals#Thread_Local_Cache_.28tcache.29

    While this malloc is aware of multiple threads, that's pretty much 
the extent of its awareness - it knows there are multiple threads. There 
is no code in this malloc to optimize it for NUMA architectures, 
coordinate thread locality, sort threads by core, etc. It is assumed 
that the kernel will handle those issues sufficiently well.

    Each thread has a thread-local variable that remembers which arena 
it last used. If that arena is in use when a thread needs to use it the 
thread will block to wait for the arena to become free. If the thread 
has never used an arena before then it may try to reuse an unused one, 
create a new one, or pick the next one on the global list.

    Each thread has a per-thread cache (called the tcache) containing a 
small collection of chunks which can be accessed without needing to lock 
an arena. These chunks are stored as an array of singly-linked lists, 
like fastbins, but with links pointing to the payload (user area) not 
the chunk header. Each bin contains one size chunk, so the array is 
indexed (indirectly) by chunk size. Unlike fastbins, the tcache is 
limited in how many chunks are allowed in each bin (tcache_count). If 
the tcache bin is empty for a given requested size, the next larger 
sized chunk is not used (could cause internal fragmentation), instead 
the fallback is to use the normal malloc routines i.e. locking the 
thread's arena and working from there.

[MaWin (Gast)]

Ich würde es mal mit Valgrind laufen lassen.
Der sagt dir höchstwahrscheinlich genau mit Backtrace, wo dein Fehler 
ist.

[Joachim D. (Firma: JDCC) (scheppertreiber)]

valgrind ? Was ist denn das ? (bin noch ziemlich neu in Linux)

[MaWin (Gast)]

Ein sehr mächtiger Speicherfehler-Checker.
Wird bei praktisch allen Distributionen als Paket mitgeliefert.

[Oliver S. (oliverso)]

Joachim D. schrieb:
> Auf stackoverflow gefunden:

Das sind glibc-Internas, die nichts mit deinem Problem zu tun haben
Die ändern überhaupt nichts daran, daß dein Code Speicher mehrfach 
freigibt. Das fällt halt dort auf.

Was du auch mal überprüfen könntest, ist, ob die Arrays überlaufen. Das 
könnte tmp überschreiben.

Oliver

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Also Folgefehler ... kann sein.

Ich schau nochmal alles durch.

valgrind  schau ich mir mal an.

Vielen Dank schon mal.

[Georg A. (georga)]

Joachim D. schrieb:
> valgrind  schau ich mir mal an.

https://valgrind.org/docs/manual/quick-start.html#quick-start.mcrun

Einfach nur ein paar Zeichen vor den Programmaufruf. Und dann hat man 
oft länger was an den Meckereien zu knabbern ;)

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Georg A. schrieb:
> https://valgrind.org/docs/manual/quick-start.html#quick-start.mcrun

Das muß ich nur irgendwie über http zum Laufen kriegen oder den
Kram von der cmdline aufrufen ... Wir schon gehen ;)

[MaWin (Gast)]

Joachim D. schrieb:
> Das muß ich nur irgendwie über http zum Laufen kriegen

Was hat das mit HTTP zu tun?
Du musst lediglich statt `meinprogramm` nun `valgrind meinprogramm` 
starten.

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Ist ein Webserver. Keine Kommandozeile. Habe eine zum Test eingebaut.

==6731== Memcheck, a memory error detector
==6731== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==6731== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright 
info
==6731== Command: ../../bin/uquery.x --test
==6731==
==6736== Invalid read of size 4
==6736==    at 0xA3BECFB: fclose@@GLIBC_2.2.5 (iofclose.c:48)
==6736==    by 0x10F766: DoCmd (httpsrv.c:177)
==6736==    by 0x10F46F: HttpServer (httpsrv.c:115)
==6736==    by 0x12CD26: main (uquery.c:124)
==6736==  Address 0xa58cbb0 is 0 bytes inside a block of size 472 free'd
==6736==    at 0xA32127F: free (in 
/usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==6736==    by 0xA3BEDA6: _IO_deallocate_file (libioP.h:862)
==6736==    by 0xA3BEDA6: fclose@@GLIBC_2.2.5 (iofclose.c:74)
==6736==    by 0x112F73: GetInputData (input.c:201)
==6736==    by 0x10F757: DoCmd (httpsrv.c:175)
==6736==    by 0x10F46F: HttpServer (httpsrv.c:115)
==6736==    by 0x12CD26: main (uquery.c:124)
...
==6731== LEAK SUMMARY:
==6731==    definitely lost: 0 bytes in 0 blocks
==6731==    indirectly lost: 0 bytes in 0 blocks
==6731==      possibly lost: 0 bytes in 0 blocks
==6731==    still reachable: 2,525 bytes in 7 blocks
==6731==         suppressed: 0 bytes in 0 blocks
==6731== Reachable blocks (those to which a pointer was found) are not 
shown.
==6731== To see them, rerun with: --leak-check=full 
--show-leak-kinds=all
==6731==
==6731== For lists of detected and suppressed errors, rerun with: -s
==6731== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Schaut so aus als wäre das Problem, daß ich einen globalen Filehandle
FILE* verwende, ich vermute mal der sollte nur in dem angeschubsten
Prozess gültig sein. Ich baue das halt um ;(

Die Ausgabe habe ich gekürzt, sonst kommt formerror_text_long

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Ja, das war's ;)

Die Meldung ist weg.

Vielen Dank nochmal an alle ! Ich hatte ein file dessen FILE*
global definiert war, in dem neuen Prozess geöffnet und später
dann wieder geschlossen. Das mußte krachen ...

Was das tcache2 damit zu tun hat ? Ich liebe C

[Marco H. (damarco)]

So was macht man nicht selbst sondern verwendet Bibliotheken. Dann kann 
man sich mit der Anwendung beschäftigen und nicht mit Elementaren Kram. 
Der dir so wie es ausschaut sehr früh auf die Füße fällt.

https://www.gnu.org/software/libmicrohttpd/manual/libmicrohttpd.html

Als Beispiel welches sehr brauchbar ist....

[Georg A. (georga)]

Marco H. schrieb:
> So was macht man nicht selbst sondern verwendet Bibliotheken. Dann kann
> man sich mit der Anwendung beschäftigen und nicht mit Elementaren Kram.
> Der dir so wie es ausschaut sehr früh auf die Füße fällt.

... und dann implodiert einem halt das Framework irgendwann und man hat 
keine Ahnung, wie man überhaupt rausfindet, wo und warum das sein 
könnte. Wenn man selber Unfug macht, muss das ja nicht unbedingt im 
eigenen Code auffallen.

Nene, so ein eigenes Gebastel und das Lernen von Grundlagen des 
Debuggings sind schon wichtig.

[Joachim D. (Firma: JDCC) (scheppertreiber)]

Georg,

ich sehe das auch so.

Ich bin mit diesen "Fertig-all-inclusive-Sachen" schon ziemlich auf
die Schnauze geflogen. Nicht nochmal ;)

[MaWin (Gast)]

Georg A. schrieb:
> Nene, so ein eigenes Gebastel und das Lernen von Grundlagen des
> Debuggings sind schon wichtig.

Ja, ist schon wichtig die remote-exploitable Schwachstelle selbst 
einzubauen, wenn man sich seinen eigenen Webserver in C bastelt.

char    http_header[4096];
...
appsprintf( http_header, "%s\n", str);

Ob da ein Überlauf möglich ist? Keine Ahnung. Aber es riecht stark nach 
Fisch.

[Nop (Gast)]

Joachim D. schrieb:
> Alle free() in allen Dateien sind
> durch eine Abfrage ob der pointer != NULL ist abgesichert.

Das allein reicht aber IMO nicht, weil es eine race condition gibt. 
Pointer ist ungleich null, Thread 1 kommt durch die if-Abfrage, macht 
free(), wird aber vor dem manuellen Nullen des Pointers wegscheduled. 
Thread 2 kommt deswegen auch durch die if-Abfrage, es kommt zum double 
free.

[Joachim D. (Firma: JDCC) (scheppertreiber)]

MaWin schrieb:
> Ob da ein Überlauf möglich ist? Keine Ahnung. Aber es riecht stark nach
> Fisch.

Ist nur aus Testzwecken drin, fliegt später wieder raus.

[Marco H. (damarco)]

Joachim D. schrieb:
> Georg,
>
> ich sehe das auch so.
>
> Ich bin mit diesen "Fertig-all-inclusive-Sachen" schon ziemlich auf
> die Schnauze geflogen. Nicht nochmal ;)

Zuerst schaut man sich den Vorschlag an, dann wird man Festellen das der 
Vorschlag gar nicht so dumm war.

HTTP ist mehr als man denkt. Sein Kram wird jenseits von GET an den 
modernen Browsers schon scheitern. Wenn die Daten größer werden und der 
Browser anfängt mit Chunks ist sein Latein sowie so zu ende. Ich sehe 
das es jetzt schon der Fall ist, wenn man nur mal so in das HTTP 
Protokoll schaut welche Anforderungen selbst bei simplen Anfragen GET 
Anfragen zu erfüllen sind.

Nur den Header auseinander zu nehmen, wird ich ohne Bibliotheken nicht 
mehr alleine aufsetzen wollen. Das mag vor 20 Jahren aufgrund von 
alternativen und der noch Einfachheit des Protokolls anderes gewesen 
sein.

Das läuft vom Ansatz her gegen den Baum, ohne die Endanwendung zu 
betrachten.