C - SIGSEGV Segmentation fault

[Daniel A. (daniel-a)]

Hallo.

Ich Schreibe zurzeit auf Linux einen Webserver basierend auf 
raw-sockets, um diesen später auf Mikrocontrollern verwenden zu können. 
Ich habe bereits das ARP und das ICMP protocol implementiert. Einige 
ping abfragen funktionieren auch, aber dann stürzt das Programm ab. Ich 
finde den fehler einfach nicht, weder mit gdb noch mit valgrind konnte 
ich die Uhrsache finden.

Das Projekt:
https://github.com/Daniel-Abrecht/DPA-UCS/tree/4572546dcaf9bae11d4227ffe5d25165ef1ed3f1

Ausgabe von Valgrind:

daniel@daniel-Inspiron-7720:/projects/DPA-UCS$ sudo valgrind ./bin/linux 
==4244== Memcheck, a memory error detector
==4244== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==4244== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==4244== Command: ./bin/linux
==4244== 
socket: ok
==4244== Syscall param socketcall.bind(my_addr.sa_data) points to uninitialised byte(s)
==4244==    at 0x4F3FC07: bind (syscall-template.S:81)
==4244==    by 0x400DD6: DPAUCS_ethInit (linux.c:90)
==4244==    by 0x400FBC: DPAUCS_init (server.c:29)
==4244==    by 0x400A4E: main (main.c:12)
==4244==  Address 0xfff0003e4 is on thread 1's stack
==4244==  in frame #1, created by DPAUCS_ethInit (linux.c:63)
==4244== 
wlan0 60:36:dd:26:4a:bc
send: 98 98 ok
send: 98 98 ok
send: 98 98 ok
send: 98 98 ok
send: 98 98 ok
send: 98 98 ok
==4244== Invalid read of size 8
==4244==    at 0x4028E0: IPv4_transmissionEnd (ip.c:69)
==4244==    by 0x40247D: DPAUCS_icmp_handler (icmp.c:18)
==4244==    by 0x402DD7: IPv4_handler (ip.c:228)
==4244==    by 0x402E49: DPAUCS_ip_handler (ip.c:248)
==4244==    by 0x4014DB: DPAUCS_doNextTask (server.c:143)
==4244==    by 0x400A76: main (main.c:20)
==4244==  Address 0x40 is not stack'd, malloc'd or (recently) free'd
==4244== 
==4244== 
==4244== Process terminating with default action of signal 11 (SIGSEGV)
==4244==  Access not within mapped region at address 0x40
==4244==    at 0x4028E0: IPv4_transmissionEnd (ip.c:69)
==4244==    by 0x40247D: DPAUCS_icmp_handler (icmp.c:18)
==4244==    by 0x402DD7: IPv4_handler (ip.c:228)
==4244==    by 0x402E49: DPAUCS_ip_handler (ip.c:248)
==4244==    by 0x4014DB: DPAUCS_doNextTask (server.c:143)
==4244==    by 0x400A76: main (main.c:20)
==4244==  If you believe this happened as a result of a stack
==4244==  overflow in your program's main thread (unlikely but
==4244==  possible), you can try to increase the size of the
==4244==  main thread stack using the --main-stacksize= flag.
==4244==  The main thread stack size used in this run was 8388608.
==4244== 
==4244== HEAP SUMMARY:
==4244==     in use at exit: 0 bytes in 0 blocks
==4244==   total heap usage: 0 allocs, 0 frees, 0 bytes allocated
==4244== 
==4244== All heap blocks were freed -- no leaks are possible
==4244== 
==4244== For counts of detected and suppressed errors, rerun with: -v
==4244== Use --track-origins=yes to see where uninitialised values come from
==4244== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)

Ausgabe von Ping:

daniel@daniel-Inspiron-7720:/datas/projects/DPA-UCS$ ping 192.168.1.29
PING 192.168.1.29 (192.168.1.29) 56(84) bytes of data.
64 bytes from 192.168.1.29: icmp_seq=1 ttl=64 time=35.1 ms
64 bytes from 192.168.1.29: icmp_seq=1 ttl=64 time=35.5 ms (DUP!)
64 bytes from 192.168.1.29: icmp_seq=2 ttl=64 time=57.4 ms
64 bytes from 192.168.1.29: icmp_seq=2 ttl=64 time=57.9 ms (DUP!)
64 bytes from 192.168.1.29: icmp_seq=3 ttl=64 time=80.4 ms
64 bytes from 192.168.1.29: icmp_seq=3 ttl=64 time=80.8 ms (DUP!)
^C
--- 192.168.1.29 ping statistics ---
5 packets transmitted, 3 received, +3 duplicates, 40% packet loss, time 4011ms
rtt min/avg/max/mdev = 35.107/57.925/80.882/18.514 ms

Verwendeter Compiler:

daniel@daniel-Inspiron-7720:~$ gcc --version
gcc (Ubuntu 4.9.2-10ubuntu13) 4.9.2
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

Ich kann mir einfach nicht erklären, wie die Variable 
currentTransmission.to in der Datei src/server/protocol/ip.c auf Zeile 
69 den Wert 0x40 haben konnte.

Wo hab ich diesmal einen Fehler gemacht? Meine debugging fähigkeiten 
stossen hier an ihre Grenzen.

[Mark B. (markbrandis)]

Wo wird denn

static stream_t* IPv4_transmissionBegin( void* from, void** to, uint8_t type )

aufgerufen? Bzw. wo werden die drei Parameter, die an die Funktion zu 
übergeben sind, auf ihre richtigen Werte gesetzt? Hab es auf die 
Schnelle durch Ankucken der .c Dateien im Browser nicht gefunden.

Bzw. es könnte hier drin versteckt sein:

      while(true){
        if(
            !(*handler)(
              &f->info->src,
              &f->info->dest,
              &IPv4_transmissionBegin,
              &IPv4_transmissionEnd,
              f->offset,
              f->length,
              payload,
              !(fragment.flags & IPv4_FLAG_MORE_FRAGMENTS)
            )

Aber da mag ich nicht drin rumsuchen. Muss man den Code so verworren 
schreiben, oder geht es auch normal? ;-)
Welchen Übergabewert Parameter an eine Funktion haben bzw. wo genau die 
Zuweisung passiert, sollte in aller Regel auf einen Blick erfassbar 
sein. Wenn das nicht der Fall ist, macht man sich das Leben damit 
unnötig schwer.

Da das Ganze im Moment eh auf Linux läuft, hindert Dich ja nichts daran 
Debug-Informationen auf die Konsole und/oder in eine Datei auszugeben. 
Das wäre so das Erste, was ich persönlich machen würde.

[Daniel A. (daniel-a)]

Mark Brandis schrieb:
> Wo wird denn
>

> static stream_t* IPv4_transmissionBegin( void* from, void** to, uint8_t 
> type )
>

>
> aufgerufen? Bzw. wo werden die drei Parameter, die an die Funktion zu
> übergeben sind, auf ihre richtigen Werte gesetzt? Hab es auf die
> Schnelle durch Ankucken der .c Dateien im Browser nicht gefunden.

Das ist in src/server/protocol/icmp.c

void* ret[] = { from, 0 };
stream_t* stream = (*begin)( to, ret, IP_PROTOCOL_ICMP );
DPAUCS_stream_referenceWrite( stream, payload, length );
(*end)();

> Bzw. es könnte hier drin versteckt sein:
>

>       while(true){
>         if(
>             !(*handler)(
>               &f->info->src,
>               &f->info->dest,
>               &IPv4_transmissionBegin,
>               &IPv4_transmissionEnd,
>               f->offset,
>               f->length,
>               payload,
>               !(fragment.flags & IPv4_FLAG_MORE_FRAGMENTS)
>             )
>

Ist es irgendwie auch. Handler ist auf DPAUCS_icmp_handler gesetzt. 
Kompt von src/main.c:

DPAUCS_addIpProtocolHandler(IP_PROTOCOL_ICMP,&DPAUCS_icmp_handler);

Villeicht sollte ich das in DPAUCS_icmpInit() umbenenenn...

> Aber da mag ich nicht drin rumsuchen. Muss man den Code so verworren
> schreiben, oder geht es auch normal? ;-)

Nunja, später soll handler auch &DPAUCS_tcp_handler oder 
&DPAUCS_udp_handler sein können und ich will mir die möglichkeit 
offenhalten später auch z.B. IPv6 zu implementieren, ohne später höhere 
layer verändern zu müssen. Ausserdem will ich icmp oder tcp support 
einfach deaktivieren können, ohne viel am Code ändern zu müssen.
Was wäre den hier gut und normal? Wäre ein switch case und week symbols 
besser? Oder ein switch case mit Makros?

> Welchen Übergabewert Parameter an eine Funktion haben bzw. wo genau die
> Zuweisung passiert, sollte in aller Regel auf einen Blick erfassbar
> sein. Wenn das nicht der Fall ist, macht man sich das Leben damit
> unnötig schwer.

Ist dass den nicht immer schwierig wenn man ein Projekt zum ersten mal 
sieht?

> Da das Ganze im Moment eh auf Linux läuft, hindert Dich ja nichts daran
> Debug-Informationen auf die Konsole und/oder in eine Datei auszugeben.
> Das wäre so das Erste, was ich persönlich machen würde.

Stimmt, das werde ich morgen mal versuchen.

[Daniel A. (daniel-a)]

Debug ausgaben auf der Konsole auszugeben hat mir nur Dinge gezeigt, die 
ich sowiso schon wusste. Mit gdb konnte ich dass problem jetzt jedoch 
eindeutig identifizieren. Es ist ein Buffer Overflow.

Ausschnitt aus gdb:

Hardware watchpoint 2: currentTransmission.from

Old value = (void *) 0x7fffffffe3bc
New value = (void *) 0x1
0x000000000040189d in DPAUCS_stream_referenceWrite (stream=0x6a7120 <outputStream>, 
    p=0x6a7264 <packetInputBuffer+36>, size=64) at src/server/stream.c:32
32    BUFFER_PUT(stream->buffer_buffer,entry);
(gdb) backtrace
#0  0x000000000040189d in DPAUCS_stream_referenceWrite (stream=0x6a7120 <outputStream>, 
    p=0x6a7264 <packetInputBuffer+36>, size=64) at src/server/stream.c:32
#1  0x0000000000402473 in DPAUCS_icmp_handler (from=0x7fffffffe3b0, to=0x7fffffffe3bc, 
    begin=0x402552 <IPv4_transmissionBegin>, end=0x402738 <IPv4_transmissionEnd>, offset=0, length=64, 
    payload=0x6a7264 <packetInputBuffer+36>, last=true) at src/server/protocol/icmp.c:17
#2  0x0000000000403102 in IPv4_handler (info=0x7fffffffe430, ip=0x6a7250 <packetInputBuffer+16>)
    at src/server/protocol/ip.c:279
#3  0x0000000000403174 in DPAUCS_ip_handler (info=0x7fffffffe430) at src/server/protocol/ip.c:299
#4  0x00000000004014dc in DPAUCS_doNextTask () at src/server/server.c:143
#5  0x0000000000400a77 in main () at src/main.c:20
(gdb) print *stream->buffer_buffer
$3 = {buffer = 0x6a8100 <outputStreamBufferBuffer_buffer>, size_mask = 63, read_offset = 6, write_offset = 6}
(gdb) print (stream->buffer_buffer)->write_offset & (stream->buffer_buffer)->size_mask 
$5 = 6
(gdb) print sizeof(outputStreamBufferBuffer_buffer)
$6 = 144
(gdb) print sizeof(outputStreamBufferBuffer_buffer) / sizeof(*outputStreamBufferBuffer_buffer)
$7 = 6

BUFFER_PUT:

#define BUFFER_PUT(buf,x) \
  do { \
    (buf)->buffer[ (buf)->write_offset & (buf)->size_mask ] = x; \
    (buf)->write_offset++; \
  } while(0)

Der Fehler war hier:

#define DEFINE_BUFFER(B,T,name,size) \
  static B name ## _buffer[size]; \
  T name = { name ## _buffer, (1<<size)-1, 0, 0 }

Korrekt wäre gewesen:

#define DEFINE_BUFFER(B,T,name,size) \
  static B name ## _buffer[1<<size]; \
  T name = { name ## _buffer, (1<<size)-1, 0, 0 }

[Mark B. (markbrandis)]

Daniel A. schrieb:
> Das ist in src/server/protocol/icmp.c

Bei einer static-Funktion fände ich es sinnvoller, wenn man dies auch im 
gleichen Modul wiederfinden würde.

[Daniel A. (daniel-a)]

Ich verstehe das static Keyword bei globalen Symbolen als prinzip der 
Sichtbarkei derselben, welches dazu benutzt werden kann die direkte 
verwendung des Symbols in anderen Compilationsunits zu verhindern. Wo 
die Variable verwendet wird spielt keine Rolle, wichtig ist, dass das 
Symbol nur in der selben Compilationsunit verwendet werden kann und 
damit die versehentliche Mehrfachdefinition von Symbolen mit 
unterschiedlichem zweck vermieden wird. Ich weiss, das es in diesem fall 
nicht notwendig wäre, aber ich versuche meine Designgrundsätze 
konsistent beizubehalten. Ausserdem kommt man so nicht auf die Idee nach 
dem Symbol an Orten zu suchen, wo es nicht vorkommen kann.