On Mon, 3 Apr 2023 at 15:56, Stephen Hemminger <step...@networkplumber.org> wrote: > > On Sun, 2 Apr 2023 23:58:52 +0100 > Luca Boccassi <bl...@debian.org> wrote: > > > On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker <bernha...@mailbox.org> > > wrote: > > > > > > Dear Maintainer, > > > I tried to find out where exactly the stack smashing takes place. > > > And found the ioctl SIOCCHGTUNNEL did write more than the 52 bytes > > > allocated in variable old_p, by that overwriting the stack canary. > > > > > > Kind regards, > > > Bernhard > > > > > > > > > (gdb) > > > 0x000055555557589f 62 { > > > 1: x/i $pc > > > => 0x55555557589f <parse_args+31>: mov %fs:0x28,%rax > > > (gdb) > > > 0x00005555555758a8 62 { > > > 1: x/i $pc > > > => 0x5555555758a8 <parse_args+40>: mov %rax,0x68(%rsp) > > > (gdb) print/x $rax > > > $1 = 0xbf9b77d893accd00 > > > (gdb) print/x $rsp + 0x68 > > > $2 = 0x7fffffffea28 > > > > > > > > > (gdb) > > > 0x00007ffff7e575f5 120 in ../sysdeps/unix/syscall-template.S > > > 1: x/i $pc > > > => 0x7ffff7e575f5 <ioctl+5>: syscall > > > 2: /x *(uint64_t*)0x7fffffffea28 = 0xbf9b77d893accd00 > > > (gdb) bt > > > #0 0x00007ffff7e575f5 in ioctl () at > > > ../sysdeps/unix/syscall-template.S:120 > > > #1 0x0000555555578230 in tnl_get_ioctl (basedev=0x7fffffffee8f "gre1", > > > p=<optimized out>) at tunnel.c:77 > > > #2 0x0000555555576243 in parse_args (argc=9, argv=0x7fffffffec50, > > > cmd=35315, p=0x7fffffffea70) at iptunnel.c:181 > > > #3 0x00005555555762fb in do_add (cmd=35315, argc=<optimized out>, > > > argv=<optimized out>) at iptunnel.c:260 > > > #4 0x000055555556258b in do_cmd (argv0=0x7fffffffee81 "tunnel", argc=11, > > > argv=0x7fffffffec40) at ip.c:133 > > > #5 0x0000555555561fc2 in main (argc=12, argv=0x7fffffffec38) at ip.c:344 > > > (gdb) stepi > > > 0x00007ffff7e575f7 120 in ../sysdeps/unix/syscall-template.S > > > 1: x/i $pc > > > => 0x7ffff7e575f7 <ioctl+7>: cmp $0xfffffffffffff001,%rax > > > 2: /x *(uint64_t*)0x7fffffffea28 = 0x200000000000000 > > > > > > (gdb) print &old_p > > > $7 = (struct ip_tunnel_parm *) 0x7fffffffe9f0 > > > (gdb) print sizeof(old_p) > > > $8 = 52 > > > (gdb) print/x 0x7fffffffe9f0 + 52 > > > $9 = 0x7fffffffea24 > > > > > > (gdb) list iptunnel.c:181 > > > 178 if (cmd == SIOCCHGTUNNEL && count == 0) { > > > 179 struct ip_tunnel_parm old_p = {}; > > > 180 > > > 181 if (tnl_get_ioctl(*argv, &old_p)) > > > 182 return -1; > > > > Hi David and Stephen, > > > > To reproduce, build iproute2 with -fstack-protector-strong in CFLAGS, and > > run: > > > > ip tunnel add gre1 mode ip6gre local 2001:db8::1 remote 2001:db8::2 ttl 255 > > ip tunnel change gre1 mode gre local 192.168.0.0 remote 192.168.0.1 ttl 255 > > > > You'll get: > > > > *** stack smashing detected ***: terminated > > Aborted > > > > This happens because iproute2 just assumes the tunnel is ipv4, but the > > kernel "knows" it's actually ip6gre so when calling the SIOCGETTUNNEL > > ioctl it writes back a struct ip6_tnl_parm2 into the struct > > ip_tunnel_parm which is smaller, so the stack gets overwritten. Is > > there any way to tell from userspace whether a gre is v4 or v6 before > > doing an ioctl? The ioctls don't take/return a size parameter as far > > as I can see... > > Is setting IPv4 addresses on an IPv6 tunnel even a valid operation? > Assuming the ioctl to get is fixed, is there a reason to allow it? > > One more reason netlink is better than ioctl.
I don't know, I don't really know anything about GRE, but if it isn't, it should be rejected earlier, rather than overwriting the stack, which seems dangerous.