On 13 March 2015 at 16:17, Martin Pieuchot <[email protected]> wrote: > On 12/03/15(Thu) 20:02, Mike Belopuhov wrote: >> 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: > > Required for what? What are you trying to do? > >> > 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. > > Who is the sender and who is the peer? The original bug report was > using: > > # route add -inet 213.154.229.23 -link -iface pcn0 > > To add an (incomplete) ARP entry on the pcn0 interface. This ARP entry > will then be overwritten through WHO-HAS and lose its static flag. > That's why a "arp info overwritten for" messages appears in the dmesg. > > Are you describing the same situation and implying that the correct way > to configure such setup is to add "-llinfo" option? >
It appears so. At least on 5.7.
