fln / addrwatch

A tool similar to arpwatch for IPv4/IPv6 and ethernet address pairing monitoring.
GNU General Public License v3.0
184 stars 30 forks source link

addrwatch_stdout segfaults on first update #26

Open jefferyto opened 3 years ago

jefferyto commented 3 years ago

While working on the addrwatch (1.0.2) package for OpenWrt (openwrt/packages#15899) I tried running addrwatch_stdout at the command line (with addrwatch running as well). I was testing using an armvirt-32 snapshot image in qemu (like most architectures supported by OpenWrt, this uses musl instead of glibc).

addrwatch_stdout would stay running for a while (with no output), then crash with a segfault. From the variable delay my guess is that it crashed when trying to output the first update from addrwatch.

t-w commented 3 years ago

I have the same issue - and it concerns also addrwatch_syslog (see: https://github.com/fln/addrwatch/issues/25 and https://github.com/openwrt/packages/pull/15899#issuecomment-863887677 - I have exactly the same problem with it).

It seems that the problem is with formatting the log string on ARM devices (RaspberryPi, an OpenWRT router etc.) - when compiling on RaspberryPi, I am getting warnings:

gcc -Wall -Wextra -Wno-unused-parameter -g -O2   -o addrwatch addrwatch.o util.o parse.o check.o process.o base64.o storage.o mcache.o output_flatfile.o output_sqlite.o output_shm.o -levent -lrt -lpcap 
gcc -DHAVE_CONFIG_H -I.    -Wall -Wextra -Wno-unused-parameter -g -O2 -MT addrwatch_stdout.o -MD -MP -MF .deps/addrwatch_stdout.Tpo -c -o addrwatch_stdout.o addrwatch_stdout.c
addrwatch_stdout.c: In function ‘process_entry’:
addrwatch_stdout.c:19:12: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘uint64_t {aka long long unsigned int}’ [-Wformat=]
  printf("%lu %s %u %s %s %s\n", e->timestamp, e->interface, e->vlan_tag,
            ^
mv -f .deps/addrwatch_stdout.Tpo .deps/addrwatch_stdout.Po
gcc -DHAVE_CONFIG_H -I.    -Wall -Wextra -Wno-unused-parameter -g -O2 -MT shm_client.o -MD -MP -MF .deps/shm_client.Tpo -c -o shm_client.o shm_client.c
mv -f .deps/shm_client.Tpo .deps/shm_client.Po
gcc -Wall -Wextra -Wno-unused-parameter -g -O2   -o addrwatch_stdout addrwatch_stdout.o shm_client.o  -lrt -lpcap 
gcc -DHAVE_CONFIG_H -I.    -Wall -Wextra -Wno-unused-parameter -g -O2 -MT addrwatch_syslog.o -MD -MP -MF .deps/addrwatch_syslog.Tpo -c -o addrwatch_syslog.o addrwatch_syslog.c
addrwatch_syslog.c: In function ‘process_entry’:
addrwatch_syslog.c:21:22: warning: format ‘%lu’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘uint64_t {aka long long unsigned int}’ [-Wformat=]
  syslog(LOG_INFO, "%lu %s %u %s %s %s", e->timestamp, e->interface,
                      ^

The problem is with the first number (timestamp / e->timestamp). It is declared as uint64_t but formatted as %lu - what is correct eg. on Linux x86_64 - but is not correct on ARM devices. Try running:

#include <stdio.h>
#include <stdint.h>

int main()
{
    printf ( "Size of uint64_t:           %d\n" 
             "Size of long unsigned:      %d\n"
             "Size of long long unsigned: %d\n", 
             sizeof(uint64_t),
             sizeof(unsigned long),
             sizeof(unsigned long long) );    
    return 0;
}

The result on x86_64:

Size of uint64_t:           8
Size of long unsigned:      8
Size of long long unsigned: 8

but on RaspberryPi:

Size of uint64_t:           8
Size of long unsigned:      4
Size of long long unsigned: 8

Changing the format of e->timestamp to %llu should fix this for all systems (I think - but of course you may want to make sure...).