Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.debian.kernel > #78464 > unrolled thread

Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash

Started byRobin <imer@imer.cc>
First post2023-03-10 14:30 +0100
Last post2023-04-10 19:40 +0200
Articles 9 — 5 participants

Back to article view | Back to linux.debian.kernel


Contents

  Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Robin <imer@imer.cc> - 2023-03-10 14:30 +0100
    Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Bernhard Übelacker <bernhardu@mailbox.org> - 2023-03-25 01:40 +0100
      Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Luca Boccassi <bluca@debian.org> - 2023-04-03 01:10 +0200
        Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Stephen Hemminger <stephen@networkplumber.org> - 2023-04-03 17:00 +0200
          Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Luca Boccassi <bluca@debian.org> - 2023-04-03 17:10 +0200
        Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Stephen Hemminger <stephen@networkplumber.org> - 2023-04-03 17:30 +0200
          Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash David Ahern <dsahern@kernel.org> - 2023-04-04 05:00 +0200
            Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Stephen Hemminger <stephen@networkplumber.org> - 2023-04-05 04:30 +0200
            Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash Stephen Hemminger <stephen@networkplumber.org> - 2023-04-10 19:40 +0200

#78464 — Bug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash

FromRobin <imer@imer.cc>
Date2023-03-10 14:30 +0100
SubjectBug#1032642: iproute2: ip tunnel change ip6gre to gre crashes with stack smash
Message-ID<G7KYO-bvAc-3@gated-at.bofh.it>
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  libbpf0                1:0.3-2
ii  libbsd0                0.11.3-1
ii  libc6                  2.31-13+deb11u5
ii  libcap2                1:2.44-1
ii  libcap2-bin            1:2.44-1
ii  libdb5.3               5.3.28+dfsg1-0.8
ii  libelf1                0.183-1
ii  libmnl0                1.0.4-3
ii  libselinux1            3.1-3
ii  libxtables12           1.8.7-1

Versions of packages iproute2 recommends:
pn  libatm1  <none>

Versions of packages iproute2 suggests:
pn  iproute2-doc  <none>

-- debconf information:
  iproute2/setcaps: false

[toc] | [next] | [standalone]


#78590

FromBernhard Übelacker <bernhardu@mailbox.org>
Date2023-03-25 01:40 +0100
Message-ID<Gd06R-eSdj-1@gated-at.bofh.it>
In reply to#78464
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;

[toc] | [prev] | [next] | [standalone]


#78679

FromLuca Boccassi <bluca@debian.org>
Date2023-04-03 01:10 +0200
Message-ID<GgeZH-gWhy-1@gated-at.bofh.it>
In reply to#78590
On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker <bernhardu@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...

[toc] | [prev] | [next] | [standalone]


#78683

FromStephen Hemminger <stephen@networkplumber.org>
Date2023-04-03 17:00 +0200
Message-ID<GgtP3-h5zS-1@gated-at.bofh.it>
In reply to#78679
On Sun, 2 Apr 2023 23:58:52 +0100
Luca Boccassi <bluca@debian.org> wrote:

> On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker <bernhardu@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.

[toc] | [prev] | [next] | [standalone]


#78684

FromLuca Boccassi <bluca@debian.org>
Date2023-04-03 17:10 +0200
Message-ID<GgtYJ-h5Tc-1@gated-at.bofh.it>
In reply to#78683
On Mon, 3 Apr 2023 at 15:56, Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Sun, 2 Apr 2023 23:58:52 +0100
> Luca Boccassi <bluca@debian.org> wrote:
>
> > On Sat, 25 Mar 2023 at 00:39, Bernhard Übelacker <bernhardu@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.

[toc] | [prev] | [next] | [standalone]


#78685

FromStephen Hemminger <stephen@networkplumber.org>
Date2023-04-03 17:30 +0200
Message-ID<Ggui5-h5ZW-7@gated-at.bofh.it>
In reply to#78679
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?

[toc] | [prev] | [next] | [standalone]


#78687

FromDavid Ahern <dsahern@kernel.org>
Date2023-04-04 05:00 +0200
Message-ID<GgF3P-hcBr-3@gated-at.bofh.it>
In reply to#78685
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.

[toc] | [prev] | [next] | [standalone]


#78693

FromStephen Hemminger <stephen@networkplumber.org>
Date2023-04-05 04:30 +0200
Message-ID<Gh14l-hqzO-1@gated-at.bofh.it>
In reply to#78687
On Mon, 3 Apr 2023 20:47:01 -0600
David Ahern <dsahern@kernel.org> 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 

[toc] | [prev] | [next] | [standalone]


#78723

FromStephen Hemminger <stephen@networkplumber.org>
Date2023-04-10 19:40 +0200
Message-ID<Gj3EJ-18dy-1@gated-at.bofh.it>
In reply to#78687
On Mon, 3 Apr 2023 20:47:01 -0600
David Ahern <dsahern@kernel.org> 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??

[toc] | [prev] | [standalone]


Back to top | Article view | linux.debian.kernel


csiph-web