Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
On Mon, 3 Apr 2023 20:47:01 -0600 David Ahern wrote: > On 4/3/23 9:24 AM, Stephen Hemminger wrote: > > ted > >> > >> 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... > > > > Ip uses and IPv4 UDP socket when it thinks it is talking to GRE. > > And a IPv6 UDP socket when it is talking to GRE6. > > > > So the kernel could check and error out? > > > > Does seem like a kernel bug and a well known design flaw in ioctl > interface (assuming buffer of a specific size). The best iproute2 can do > is have `old_p` be a larger size (e.g., ip6_tnl_parm2) to avoid the > overrun, but then the result is nonsense with no way for it no an ipv6 > struct was passed back. The crash at least indicates something is off. I started to look into redoing the whole 'ip tunnel XXX' as just a remapping of arguments and calling the equivalent 'ip link ... type YYY' and it is doable for the basic stuff. Then starting looking at the Potential Router List (PRL) stuff. Looks like this is only supported through ioctl(). Definitely a dusty dark corner of networking code with rarely used features. Plus things like, the code to get PRL will allow bigger get if called from root vs non-root user??
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
On Mon, 3 Apr 2023 20:47:01 -0600 David Ahern wrote: > On 4/3/23 9:24 AM, Stephen Hemminger wrote: > > ted > >> > >> 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... > > > > Ip uses and IPv4 UDP socket when it thinks it is talking to GRE. > > And a IPv6 UDP socket when it is talking to GRE6. > > > > So the kernel could check and error out? > > > > Does seem like a kernel bug and a well known design flaw in ioctl > interface (assuming buffer of a specific size). The best iproute2 can do > is have `old_p` be a larger size (e.g., ip6_tnl_parm2) to avoid the > overrun, but then the result is nonsense with no way for it no an ipv6 > struct was passed back. The crash at least indicates something is off. Actually any change tunnel can have similar issues where the tunnel is of one type and the request wants to change parameters. The two structs (ip_param and ip6_tunnel_param) are different enough that getting the incorrect type will be complete garbage. There doesn't seem to be a good way to identify the tunnel type. The only way I can see is to look at the link type (ifi_type) but this is ARPHRD_XXX value and not the ip protocol. The other way would be to query link info (with netlink) and make sure that IFLA_INFO_KIND (in IFLA_LINKINFO) matches when changing. Or maybe get rid of the ip tunnel command and just use ip link which is all netlink based. The iptunnel stuff was introduced long ago when the only way to make tunnels was with ioctl. Now you can do same operations with ip link. to
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
On 4/3/23 9:24 AM, Stephen Hemminger wrote: > ted >> >> 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... > > Ip uses and IPv4 UDP socket when it thinks it is talking to GRE. > And a IPv6 UDP socket when it is talking to GRE6. > > So the kernel could check and error out? > Does seem like a kernel bug and a well known design flaw in ioctl interface (assuming buffer of a specific size). The best iproute2 can do is have `old_p` be a larger size (e.g., ip6_tnl_parm2) to avoid the overrun, but then the result is nonsense with no way for it no an ipv6 struct was passed back. The crash at least indicates something is off.
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
ted > > 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... Ip uses and IPv4 UDP socket when it thinks it is talking to GRE. And a IPv6 UDP socket when it is talking to GRE6. So the kernel could check and error out?
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
On Mon, 3 Apr 2023 at 15:56, Stephen Hemminger
wrote:
>
> On Sun, 2 Apr 2023 23:58:52 +0100
> Luca Boccassi wrote:
>
> > On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker
> > 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)
> > > 0x5557589f 62 {
> > > 1: x/i $pc
> > > => 0x5557589f : mov%fs:0x28,%rax
> > > (gdb)
> > > 0x555758a8 62 {
> > > 1: x/i $pc
> > > => 0x555758a8 : mov%rax,0x68(%rsp)
> > > (gdb) print/x $rax
> > > $1 = 0xbf9b77d893accd00
> > > (gdb) print/x $rsp + 0x68
> > > $2 = 0x7fffea28
> > >
> > >
> > > (gdb)
> > > 0x77e575f5 120 in ../sysdeps/unix/syscall-template.S
> > > 1: x/i $pc
> > > => 0x77e575f5 :syscall
> > > 2: /x *(uint64_t*)0x7fffea28 = 0xbf9b77d893accd00
> > > (gdb) bt
> > > #0 0x77e575f5 in ioctl () at
> > > ../sysdeps/unix/syscall-template.S:120
> > > #1 0x55578230 in tnl_get_ioctl (basedev=0x7fffee8f "gre1",
> > > p=) at tunnel.c:77
> > > #2 0x55576243 in parse_args (argc=9, argv=0x7fffec50,
> > > cmd=35315, p=0x7fffea70) at iptunnel.c:181
> > > #3 0x555762fb in do_add (cmd=35315, argc=,
> > > argv=) at iptunnel.c:260
> > > #4 0x5556258b in do_cmd (argv0=0x7fffee81 "tunnel", argc=11,
> > > argv=0x7fffec40) at ip.c:133
> > > #5 0x55561fc2 in main (argc=12, argv=0x7fffec38) at ip.c:344
> > > (gdb) stepi
> > > 0x77e575f7 120 in ../sysdeps/unix/syscall-template.S
> > > 1: x/i $pc
> > > => 0x77e575f7 :cmp$0xf001,%rax
> > > 2: /x *(uint64_t*)0x7fffea28 = 0x200
> > >
> > > (gdb) print _p
> > > $7 = (struct ip_tunnel_parm *) 0x7fffe9f0
> > > (gdb) print sizeof(old_p)
> > > $8 = 52
> > > (gdb) print/x 0x7fffe9f0 + 52
> > > $9 = 0x7fffea24
> > >
> > > (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, _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.
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
On Sun, 2 Apr 2023 23:58:52 +0100
Luca Boccassi wrote:
> On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker
> 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)
> > 0x5557589f 62 {
> > 1: x/i $pc
> > => 0x5557589f : mov%fs:0x28,%rax
> > (gdb)
> > 0x555758a8 62 {
> > 1: x/i $pc
> > => 0x555758a8 : mov%rax,0x68(%rsp)
> > (gdb) print/x $rax
> > $1 = 0xbf9b77d893accd00
> > (gdb) print/x $rsp + 0x68
> > $2 = 0x7fffea28
> >
> >
> > (gdb)
> > 0x77e575f5 120 in ../sysdeps/unix/syscall-template.S
> > 1: x/i $pc
> > => 0x77e575f5 :syscall
> > 2: /x *(uint64_t*)0x7fffea28 = 0xbf9b77d893accd00
> > (gdb) bt
> > #0 0x77e575f5 in ioctl () at ../sysdeps/unix/syscall-template.S:120
> > #1 0x55578230 in tnl_get_ioctl (basedev=0x7fffee8f "gre1",
> > p=) at tunnel.c:77
> > #2 0x55576243 in parse_args (argc=9, argv=0x7fffec50,
> > cmd=35315, p=0x7fffea70) at iptunnel.c:181
> > #3 0x555762fb in do_add (cmd=35315, argc=,
> > argv=) at iptunnel.c:260
> > #4 0x5556258b in do_cmd (argv0=0x7fffee81 "tunnel", argc=11,
> > argv=0x7fffec40) at ip.c:133
> > #5 0x55561fc2 in main (argc=12, argv=0x7fffec38) at ip.c:344
> > (gdb) stepi
> > 0x77e575f7 120 in ../sysdeps/unix/syscall-template.S
> > 1: x/i $pc
> > => 0x77e575f7 :cmp$0xf001,%rax
> > 2: /x *(uint64_t*)0x7fffea28 = 0x200
> >
> > (gdb) print _p
> > $7 = (struct ip_tunnel_parm *) 0x7fffe9f0
> > (gdb) print sizeof(old_p)
> > $8 = 52
> > (gdb) print/x 0x7fffe9f0 + 52
> > $9 = 0x7fffea24
> >
> > (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, _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.
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker 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)
> 0x5557589f 62 {
> 1: x/i $pc
> => 0x5557589f : mov%fs:0x28,%rax
> (gdb)
> 0x555758a8 62 {
> 1: x/i $pc
> => 0x555758a8 : mov%rax,0x68(%rsp)
> (gdb) print/x $rax
> $1 = 0xbf9b77d893accd00
> (gdb) print/x $rsp + 0x68
> $2 = 0x7fffea28
>
>
> (gdb)
> 0x77e575f5 120 in ../sysdeps/unix/syscall-template.S
> 1: x/i $pc
> => 0x77e575f5 :syscall
> 2: /x *(uint64_t*)0x7fffea28 = 0xbf9b77d893accd00
> (gdb) bt
> #0 0x77e575f5 in ioctl () at ../sysdeps/unix/syscall-template.S:120
> #1 0x55578230 in tnl_get_ioctl (basedev=0x7fffee8f "gre1",
> p=) at tunnel.c:77
> #2 0x55576243 in parse_args (argc=9, argv=0x7fffec50, cmd=35315,
> p=0x7fffea70) at iptunnel.c:181
> #3 0x555762fb in do_add (cmd=35315, argc=,
> argv=) at iptunnel.c:260
> #4 0x5556258b in do_cmd (argv0=0x7fffee81 "tunnel", argc=11,
> argv=0x7fffec40) at ip.c:133
> #5 0x55561fc2 in main (argc=12, argv=0x7fffec38) at ip.c:344
> (gdb) stepi
> 0x77e575f7 120 in ../sysdeps/unix/syscall-template.S
> 1: x/i $pc
> => 0x77e575f7 :cmp$0xf001,%rax
> 2: /x *(uint64_t*)0x7fffea28 = 0x200
>
> (gdb) print _p
> $7 = (struct ip_tunnel_parm *) 0x7fffe9f0
> (gdb) print sizeof(old_p)
> $8 = 52
> (gdb) print/x 0x7fffe9f0 + 52
> $9 = 0x7fffea24
>
> (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, _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...
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
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)
0x5557589f 62 {
1: x/i $pc
=> 0x5557589f : mov%fs:0x28,%rax
(gdb)
0x555758a8 62 {
1: x/i $pc
=> 0x555758a8 : mov%rax,0x68(%rsp)
(gdb) print/x $rax
$1 = 0xbf9b77d893accd00
(gdb) print/x $rsp + 0x68
$2 = 0x7fffea28
(gdb)
0x77e575f5 120 in ../sysdeps/unix/syscall-template.S
1: x/i $pc
=> 0x77e575f5 :syscall
2: /x *(uint64_t*)0x7fffea28 = 0xbf9b77d893accd00
(gdb) bt
#0 0x77e575f5 in ioctl () at ../sysdeps/unix/syscall-template.S:120
#1 0x55578230 in tnl_get_ioctl (basedev=0x7fffee8f "gre1",
p=) at tunnel.c:77
#2 0x55576243 in parse_args (argc=9, argv=0x7fffec50, cmd=35315,
p=0x7fffea70) at iptunnel.c:181
#3 0x555762fb in do_add (cmd=35315, argc=, argv=) at iptunnel.c:260
#4 0x5556258b in do_cmd (argv0=0x7fffee81 "tunnel", argc=11,
argv=0x7fffec40) at ip.c:133
#5 0x55561fc2 in main (argc=12, argv=0x7fffec38) at ip.c:344
(gdb) stepi
0x77e575f7 120 in ../sysdeps/unix/syscall-template.S
1: x/i $pc
=> 0x77e575f7 :cmp$0xf001,%rax
2: /x *(uint64_t*)0x7fffea28 = 0x200
(gdb) print _p
$7 = (struct ip_tunnel_parm *) 0x7fffe9f0
(gdb) print sizeof(old_p)
$8 = 52
(gdb) print/x 0x7fffe9f0 + 52
$9 = 0x7fffea24
(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, _p))
182 return -1;
Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
Package: iproute2 Version: 5.10.0-4 Severity: normal Dear Maintainer, I just came across a "stack smashing detected" crash when changing a gre6 to a gre4 tunnel To reproduce create an ipv6 gre tunnel: > ip tunnel add gre1 mode ip6gre local 2001:db8::1 remote 2001:db8::2 ttl 255 And then attempt to change it to an ipv4 one: > ip tunnel change gre1 mode gre local 192.168.0.0 remote 192.168.0.1 ttl 255 This results in: > *** stack smashing detected ***: terminated > Aborted The inverse (changing v4 to v6) results in: > add tunnel "gre1" failed: Invalid argument Which I'm not sure if I should expect or if that's another issue, but it does not crash. I've reproduced the crash on a few other bullseye servers/vms to rule out a single broken install. I have also tested this on testing (iproute2 version 6.1.0-2) and the crash also happens there I hope this is the right place and helpful enough to action on Thanks! -- System Information: Debian Release: 11.6 APT prefers stable APT policy: (500, 'stable') Architecture: amd64 (x86_64) Kernel: Linux 5.10.0-20-amd64 (SMP w/24 CPU threads) Kernel taint flags: TAINT_PROPRIETARY_MODULE, TAINT_OOT_MODULE, TAINT_UNSIGNED_MODULE Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages iproute2 depends on: ii debconf [debconf-2.0] 1.5.77 ii libbpf01:0.3-2 ii libbsd00.11.3-1 ii libc6 2.31-13+deb11u5 ii libcap21:2.44-1 ii libcap2-bin1:2.44-1 ii libdb5.3 5.3.28+dfsg1-0.8 ii libelf10.183-1 ii libmnl01.0.4-3 ii libselinux13.1-3 ii libxtables12 1.8.7-1 Versions of packages iproute2 recommends: pn libatm1 Versions of packages iproute2 suggests: pn iproute2-doc -- debconf information: iproute2/setcaps: false
