On 12 March 2015 at 13:12, Mike Belopuhov <[email protected]> wrote: > 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.
Side note: only sender gets a permanent ARP entry however. The peer will start an expiration timer and purge it after 20 seconds.
