DHCP segfault

[kurojishi]

i'm using this click configuration with dhcp as a ethernet level router, when there is a dhcp requests the click instance crash 100% of the times

the problem seems to be in checkDHCPMsg, this element seems to assume that all packets are valid ip packets doing a jump of 28 bytes. i can change my conf to let i work but there is need of safety

//elements declaration 
require(package dhcp) 
ControlSocket(unix, /tmp/eth2.clicksocket) 
switch :: EtherSwitch; 
from_bs :: FromDevice(eth1, PROMISC true); 
to_bs :: ToDevice(eth1); 
from_net :: FromDevice(eth2, PROMISC true); 
to_net :: ToDevice(eth2); 

elementclass DHCPServer { 
$eth, $ip | 
lease :: DHCPLeaseHash($eth, $ip); 
checker :: CheckDHCPMsg; 
classifier :: DHCPClassifier(discover, requests, release, -); 
input -> checker; 
checker[1] -> output; 
checker[0] -> classifier; 
classifier[0] -> offer :: DHCPServerOffer(lease) -> output; 
classifier[1] -> ack :: DHCPServerACKorNAK(lease) -> output; 
classifier[2] -> release :: DHCPServerRelease(lease); 
classifier[3] -> sink :: Discard; 
} 

//dest address whitelist, the 2 Addresses are example and will be more than 2 
filter_from_network :: { 
filter_1 :: HostEtherFilter(00:1e:42:02:02:02, DROP_OWN false, DROP_OTHER true); 
filter_2 :: HostEtherFilter(00:1e:42:02:02:02, DROP_OWN false, DROP_OTHER true); 
input -> filter_1; 
filter_1[0], filter_2[0] -> output; 
filter_1[1] -> filter_2[1] -> sink :: Discard; 
} 

//source address whitelist, the 2 Addresses are example and will be more than 2 
filter_from_bs :: { 
filter_1 :: HostEtherFilter(00:1e:42:02:02:02, DROP_OWN true, DROP_OTHER false); 
filter_2 :: HostEtherFilter(00:1e:42:02:02:02, DROP_OWN true, DROP_OTHER false); 
input -> filter_1; 
filter_1[1] -> dhcp_1 :: DHCPServer(00:1e:42:02:02:02, 192.168.1.200) -> output; filter_2[1] -> dhcp_2 :: DHCPServer(00:1e:42:02:03:03, 192.168.1.150) -> output; 
filter_1[0] -> filter_2[0] -> sink :: Discard; 
} 

bs_queue :: Queue -> to_bs; 
net_queue :: Queue -> to_net; 

//take packet from the network, apply a whitelist of the destination addresses, remove vlan header and put them in the switch 
from_net -> filter_from_network -> [0]switch;  
// setting vlan annotation for the packets directed to the network and putting them in queue 
switch[0] -> net_queue; 

//take packet from the BS, apply a whitelist of the source addresses, remove vlan header and put them in the switch 
from_bs -> filter_from_bs -> [1]switch; 
//setting the vlan annotation for packets for the Baystation and putting them in queue 
switch[1] -> bs_queue;

[kurojishi]

updated with addictional information

[Pesa]

gdb session

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff679b550 in CheckDHCPMsg::simple_action (this=0x845950, p=0x842d30) at checkdhcpmsg.cc:46
46      if (dm->magic != DHCP_MAGIC) {

(gdb) bt
#0  0x00007ffff679b550 in CheckDHCPMsg::simple_action (this=0x845950, p=0x842d30) at checkdhcpmsg.cc:46
#1  0x0000000000555084 in Element::push (this=0x845950, port=0, p=<optimized out>) at ../lib/element.cc:2893
#2  0x00000000004d4fcf in push (this=<optimized out>, p=<optimized out>) at ../include/click/element.hh:632
#3  checked_output_push (p=<optimized out>, port=1, this=0x845a10) at ../include/click/element.hh:702
#4  HostEtherFilter::simple_action (this=0x845a10, p=<optimized out>) at ../elements/ethernet/hostetherfilter.cc:71
#5  0x0000000000555084 in Element::push (this=0x845a10, port=0, p=<optimized out>) at ../lib/element.cc:2893
#6  0x00000000005308c5 in push (p=<optimized out>, this=<optimized out>) at ../include/click/element.hh:632
#7  FromDevice::emit_packet (this=0x844d20, p=0x842d30, extra_len=<optimized out>, ts=...) at ../elements/userlevel/fromdevice.cc:442
#8  0x00000000005309bf in FromDevice_get_packet (clientdata=0x844d20 "\320\v]", pkthdr=0x7fffffffde70, data=<optimized out>) at ../elements/userlevel/fromdevice.cc:459
#9  0x00007ffff7bab44c in ?? () from /usr/lib/x86_64-linux-gnu/libpcap.so.0.8
#10 0x000000000053013a in FromDevice::selected (this=0x844d20) at ../elements/userlevel/fromdevice.cc:525
#11 0x00000000005763cd in call_selected (mask=<optimized out>, fd=8, this=0x835100) at ../lib/selectset.cc:367
#12 SelectSet::run_selects_poll (this=0x835100, thread=<optimized out>) at ../lib/selectset.cc:474
#13 0x00000000005766b9 in SelectSet::run_selects (this=0x835100, thread=0x835090) at ../lib/selectset.cc:581
#14 0x000000000056a367 in run_os (this=0x835090) at ../lib/routerthread.cc:506
#15 RouterThread::driver (this=0x835090) at ../lib/routerthread.cc:675
#16 0x00000000004aa344 in main (argc=<optimized out>, argv=<optimized out>) at click.cc:670

(gdb) f 0
#0  0x00007ffff679b550 in CheckDHCPMsg::simple_action (this=0x845950, p=0x842d30) at checkdhcpmsg.cc:46
46      if (dm->magic != DHCP_MAGIC) {

(gdb) p dm
$1 = (dhcpMessage *) 0x1c

[kohler]

Hi guys:

There's a buglet in your config. In the DHCPClassifier line

classifier :: DHCPClassifier(discover, requests, release, -);

the requests should be request. No requests are actually being processed by your config. The DHCPClassifier element didn't report an error before about this; it does now.

That said I am totally willing to believe that there are many bugs in the DHCP elements. Can you create a file containing a DHCP trace that causes the bug? -- either as a gist or in a fork? I'm in the process of creating a test file for some of this, based on your configuration. I'll check in the current version to the -packages repository.

Eddie

[Pesa]

Hi Eddie,

Thanks for noticing the typo. However in our tests scenario it's CheckDHCPMsg that crashes, thus even before packets reached the classifier.

It turns out that CheckDHCPMsg assumes incoming packets have already been marked with MarkIPHeader or CheckIPHeader and directly looks for the dhcp magic inside the packet without any safety checks such as has_network_header(). Indeed it works if I put CheckIPHeader(14) -> CheckUDPHeader before the CheckDHCPMsg.

I can understand the absence of safety checks was done for performance reasons, but I'd like to know if a crash was really intended behavior in this case. Thanks.