Skip to content

Commit

Permalink
Fixes possible crashes when dereferencing NULL Mac pointers
Browse files Browse the repository at this point in the history 
Fixes

Thread 15 "TrPoolWorker" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fffc9df4700 (LWP 314)]
0x000000000047e743 in Mac::incnDPIStats (this=0x7fff7a30a080, when=1572439200, ndpi_category=NDPI_PROTOCOL_CATEGORY_UNSPECIFIED, sent_packets=0, sent_bytes=0, sent_goodput_bytes=0, rcvd_packets=1,
    rcvd_bytes=60, rcvd_goodput_bytes=2) at /home/deri/ntopng/include/Mac.h:140
    140         stats->incnDPIStats(when, ndpi_category, sent_packets, sent_bytes, sent_goodput_bytes,
    (gdb) bt
    #0  0x000000000047e743 in Mac::incnDPIStats (this=0x7fff7a30a080, when=1572439200, ndpi_category=NDPI_PROTOCOL_CATEGORY_UNSPECIFIED, sent_packets=0, sent_bytes=0, sent_goodput_bytes=0, rcvd_packets=1, rcvd_bytes=60, rcvd_goodput_bytes=2)
        at /home/deri/ntopng/include/Mac.h:140
	#1  0x00000000004715b0 in Flow::periodic_stats_update (this=0x7ffee7685d50, user_data=0x7fffc9df3880, quick=true) at src/Flow.cpp:1154
	#2  0x000000000048a175 in host_flow_update_stats (node=0x7ffee7685d50, user_data=0x7fffc9df3880, matched=0x7fffc9df37db) at src/NetworkInterface.cpp:2647
	#3  0x00000000004596f4 in GenericHash::walk (this=0x4ab5ee0, begin_slot=0x7fffc9df387c, walk_all=true, walker=0x48a119 <host_flow_update_stats(GenericHashEntry*, void*, bool*)>, user_data=0x7fffc9df3880) at src/GenericHash.cpp:192
	#4  0x000000000048365c in NetworkInterface::walker (this=0x113a570, begin_slot=0x7fffc9df387c, walk_all=true, wtype=walker_flows, walker=0x48a119 <host_flow_update_stats(GenericHashEntry*, void*, bool*)>, user_data=0x7fffc9df3880) at src/NetworkInterface.cpp:859
	#5  0x000000000048a4e2 in NetworkInterface::periodicStatsUpdate (this=0x113a570) at src/NetworkInterface.cpp:2739
	#6  0x00000000004cb574 in ntop_periodic_stats_update (vm=0x7fff91cf4e48) at src/LuaEngine.cpp:5891
	#7  0x0000000000589a04 in luaD_precall ()
	#8  0x0000000000595025 in luaV_execute ()
	#9  0x0000000000589ccf in luaD_call ()
	#10 0x0000000000589d21 in luaD_callnoyield ()
	#11 0x000000000058913f in luaD_rawrunprotected ()
	#12 0x000000000058a03d in luaD_pcall ()
	#13 0x000000000058746f in lua_pcallk ()
	#14 0x000000000053f280 in LuaHandler::luaL_dofileM (this=0x7fff90d5bb00, execute=true) at pro/LuaHandler.cpp:32
	#15 0x00000000004d866f in __ntop_lua_handlefile (L=0x7fff91cf4e48, script_path=0x7fff8c0008e0 "/home/deri/ntopng/scripts/callbacks/interface/stats_update.lua", ex=true) at src/LuaEngine.cpp:10107
	#16 0x00000000004d99ae in LuaEngine::run_script (this=0x7fff9076c530, script_path=0x7fff8c0008e0 "/home/deri/ntopng/scripts/callbacks/interface/stats_update.lua", iface=0x113a570, load_only=false) at src/LuaEngine.cpp:11095
	#17 0x00000000004aa6c5 in ThreadedActivity::runScript (this=0xae03410, script_path=0x7fff8c0008e0 "/home/deri/ntopng/scripts/callbacks/interface/stats_update.lua", iface=0x113a570) at src/ThreadedActivity.cpp:232
	#18 0x00000000004e435c in ThreadPool::run (this=0xacc4620) at src/ThreadPool.cpp:100
	#19 0x00000000004e3f3f in doRun (ptr=0xacc4620) at src/ThreadPool.cpp:31
	#20 0x00007ffff65bd6ba in start_thread (arg=0x7fffc9df4700) at pthread_create.c:333
	#21 0x00007ffff490941d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:109
	(gdb) f 140
	#0  0x0000000000000000 in ?? ()
	(gdb) f 0
	#0  0x000000000047e743 in Mac::incnDPIStats (this=0x7fff7a30a080, when=1572439200, ndpi_category=NDPI_PROTOCOL_CATEGORY_UNSPECIFIED, sent_packets=0, sent_bytes=0, sent_goodput_bytes=0, rcvd_packets=1, rcvd_bytes=60, rcvd_goodput_bytes=2)
	    at /home/deri/ntopng/include/Mac.h:140
	    140         stats->incnDPIStats(when, ndpi_category, sent_packets, sent_bytes, sent_goodput_bytes,
	    (gdb) p stats
	    $1 = (MacStats *) 0x0
  • Loading branch information
@simonemainardi
simonemainardi committed Oct 31, 2019
1 parent 552ca89 commit 32c6eea
Showing 1 changed file with 20 additions and 17 deletions.
37 changes: 20 additions & 17 deletions src/Flow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1050,6 +1050,8 @@ void Flow::periodic_stats_update(void *user_data, bool quick) {
int16_t stats_protocol; /* The protocol (among ndpi master_ and app_) that is chosen to increase stats */
Vlan *vl;
NetworkStats *cli_network_stats;
Mac *cli_mac = get_cli_host() ? get_cli_host()->getMac() : NULL;
Mac *srv_mac = get_srv_host() ? get_srv_host()->getMac() : NULL;

if(update_flow_port_stats) {
bool dump_flow = false;
Expand Down Expand Up @@ -1138,30 +1140,30 @@ void Flow::periodic_stats_update(void *user_data, bool quick) {
if(diff_sent_bytes || diff_rcvd_bytes) {
/* Update L2 Device stats */

if(srv_host->get_mac()) {
if(srv_mac) {
#ifdef HAVE_NEDGE
srv_host->getMac()->incSentStats(tv->tv_sec, diff_rcvd_packets, diff_rcvd_bytes);
srv_host->getMac()->incRcvdStats(tv->tv_sec, diff_sent_packets, diff_sent_bytes);
srv_mac->incSentStats(tv->tv_sec, diff_rcvd_packets, diff_rcvd_bytes);
srv_mac->incRcvdStats(tv->tv_sec, diff_sent_packets, diff_sent_bytes);
#endif

if(ntop->getPrefs()->areMacNdpiStatsEnabled()) {
srv_host->getMac()->incnDPIStats(tv->tv_sec, get_protocol_category(),
diff_rcvd_packets, diff_rcvd_bytes, diff_rcvd_goodput_bytes,
diff_sent_packets, diff_sent_bytes, diff_sent_goodput_bytes);
srv_mac->incnDPIStats(tv->tv_sec, get_protocol_category(),
diff_rcvd_packets, diff_rcvd_bytes, diff_rcvd_goodput_bytes,
diff_sent_packets, diff_sent_bytes, diff_sent_goodput_bytes);

}
}

if(cli_host->getMac()) {
if(cli_mac) {
#ifdef HAVE_NEDGE
cli_host->getMac()->incSentStats(tv->tv_sec, diff_sent_packets, diff_sent_bytes);
cli_host->getMac()->incRcvdStats(tv->tv_sec, diff_rcvd_packets, diff_rcvd_bytes);
cli_mac->incSentStats(tv->tv_sec, diff_sent_packets, diff_sent_bytes);
cli_mac->incRcvdStats(tv->tv_sec, diff_rcvd_packets, diff_rcvd_bytes);
#endif

if(ntop->getPrefs()->areMacNdpiStatsEnabled()) {
cli_host->getMac()->incnDPIStats(tv->tv_sec, get_protocol_category(),
diff_sent_packets, diff_sent_bytes, diff_sent_goodput_bytes,
diff_rcvd_packets, diff_rcvd_bytes, diff_rcvd_goodput_bytes);
cli_mac->incnDPIStats(tv->tv_sec, get_protocol_category(),
diff_sent_packets, diff_sent_bytes, diff_sent_goodput_bytes,
diff_rcvd_packets, diff_rcvd_bytes, diff_rcvd_goodput_bytes);
}
}

Expand Down Expand Up @@ -1321,10 +1323,11 @@ void Flow::periodic_stats_update(void *user_data, bool quick) {
if(cli_host->get_host_pool() != srv_host->get_host_pool())
iface->topProtocolsAdd(srv_host->get_host_pool(), stats_protocol, diff_bytes);

if(cli_host->get_mac() && srv_host->getMac()) {
iface->topMacsAdd(cli_host->getMac(), stats_protocol, diff_bytes);
iface->topMacsAdd(srv_host->getMac(), stats_protocol, diff_bytes);
}
if(cli_mac)
iface->topMacsAdd(cli_mac, stats_protocol, diff_bytes);

if(srv_mac)
iface->topMacsAdd(srv_mac, stats_protocol, diff_bytes);
}

/* Just to be safe */
Expand Down Expand Up @@ -1494,7 +1497,7 @@ void Flow::update_pools_stats(const struct timeval *tv,
/* Server host */
if(srv_host
#ifdef HAVE_NEDGE
&& srv_host->getMac() && (srv_host->getMac()->locate() == located_on_lan_interface)
&& srv_mac && (srv_host->getMac()->locate() == located_on_lan_interface)
#endif
) {
srv_host_pool_id = srv_host->get_host_pool();
Expand Down

0 comments on commit 32c6eea

Please sign in to comment.