On 12 March 2015 at 11:31, Martin Pieuchot <[email protected]> wrote: > On 12/03/15(Thu) 11:12, Mike Belopuhov wrote: >> On 12 March 2015 at 10:39, Martin Pieuchot <[email protected]> wrote: >> > On 12/03/15(Thu) 09:53, Henk Jan Agteresch wrote: >> >> On Tue, 10 Mar 2015, Martin Pieuchot wrote: >> >> >> >> > >> >> > Here's a first diff that should prevent the stack smashing. Could you >> >> > run with it and tell me if the ARP entry gets overwritten as in 5.5? >> >> > >> >> >> >> Patch works for me. Arp entry gets overwritten. No more panics during >> >> network configuring. >> >> >> >> # dmesg |grep arp >> >> arpresolve: 213.154.229.23: incorrect arp information >> >> arpresolve: 213.154.229.23: incorrect arp information >> >> arpresolve: 213.154.229.23: incorrect arp information >> >> arpresolve: 213.154.229.23: incorrect arp information >> >> arpresolve: 213.154.229.23: incorrect arp information >> >> arp info overwritten for 213.154.229.23 by fe:54:00:b2:8c:98 on pcn0 >> > >> > Thanks for testing, I'll try to cook another diff to not encode the name >> > of the interface in the arp information. >> > >> > Here's the same diff with the nit pointed by krw@ fixed, any ok? >> > >> >> So how can this happen? What kind of incorrect ARP information is it? >> Where does it come from? And why didn't we see that before? > > The incorrect ARP information is the name of the interface. This is due > to the way information are encoded in sockaddr_dl, see the LLADDR() > macro. When route(8) is called with the -link argument it fills the > gateway field with link_addr(3). This field is then copied into the > kernel and passed directly to rtrequest1(RTM_ADD,). > > To understand that, look at the "so_gate" field when you run: > > $ route -nvd get default -link lo0 > > Why we didn't see that before I don't know. If you find why, I'm > interested. That's what the second diff should fix. > > My guess is that sdl_nlen is incorrectly set which would explain why > the name is now encoded and the size longer than 6. Another hypothesis > is that the sdl_data field is cleared after being set. Or we always > smashed the stack in this case... It seems the regression has been > introduced between 5.5 and 5.6 but I couldn't find it as a first glance. > > Anyway I still believe this diff is worth it as it will prevent future > bugs to smash your stack. Ok?
Looks like -llinfo is required: openbsd1:~$ sudo route -nv add -inet 213.154.229.23 -llinfo -link -iface vmx1 so_dst: inet 213.154.229.23; so_gate: link 76.6d.78.31.0.0.0.0.e.0.0.0.0.e.ad.0.0; RTM_ADD: Add Route: len 144, priority 0, table 0, pid: 0, seq 1, errno 0 flags:<UP,HOST,LLINFO,STATIC> use: 0 mtu: 0 expire: 0 locks: inits: sockaddrs: <DST,GATEWAY> 213.154.229.23 76.6d.78.31.0.0.0.0.e.0.0.0.0.e.ad.0.0 add host 213.154.229.23: gateway vmx1 This creates a valid incomplete ARP entry: openbsd1:~$ netstat -rnf inet | grep 213.154.229.23 213.154.229.23 link#2 UHLS 0 0 - 8 vmx1 openbsd1:~$ arp -na | grep 213.154.229.23 213.154.229.23 (incomplete) vmx1 expired That is later resolved through ARP WHO-HAS. Once a similar route is installed on 213.154.229.23 ping works fine.
