On 12/03/15(Thu) 20:02, Mike Belopuhov wrote: > On 12 March 2015 at 13:12, Mike Belopuhov <m...@belopuhov.com> wrote: > > On 12 March 2015 at 11:31, Martin Pieuchot <mpieuc...@nolizard.org> wrote: > >> On 12/03/15(Thu) 11:12, Mike Belopuhov wrote: > >>> On 12 March 2015 at 10:39, Martin Pieuchot <mpieuc...@nolizard.org> 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?
