Message-ID: <CANn89iKrhkLWex9Le1uCQor_sB6D7hNwNRu1nk=hY6EvpC_a2g@mail.gmail.com>
Date: Tue, 28 Nov 2023 17:15:02 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Shigeru Yoshida <syoshida@...hat.com>, davem@...emloft.net, dsahern@...nel.org, 
	kuba@...nel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net] ipv4: ip_gre: Handle skb_pull() failure in ipgre_xmit()

On Tue, Nov 28, 2023 at 5:13 PM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Tue, Nov 28, 2023 at 5:05 PM Eric Dumazet <edumazet@...gle.com> wrote:
> >
> > On Tue, Nov 28, 2023 at 4:51 PM Paolo Abeni <pabeni@...hat.com> wrote:
> > >
> > > On Tue, 2023-11-28 at 16:45 +0100, Eric Dumazet wrote:
> > > > On Sun, Nov 26, 2023 at 4:17 PM Shigeru Yoshida <syoshida@...hat.com> wrote:
> > > > >
> > > > > In ipgre_xmit(), skb_pull() may fail even if pskb_inet_may_pull() returns
> > > > > true. For example, applications can create a malformed packet that causes
> > > > > this problem with PF_PACKET.
> > > > >
> > > > > This patch fixes the problem by dropping skb and returning from the
> > > > > function if skb_pull() fails.
> > > > >
> > > > > Fixes: c54419321455 ("GRE: Refactor GRE tunneling code.")
> > > > > Signed-off-by: Shigeru Yoshida <syoshida@...hat.com>
> > > > > ---
> > > > >  net/ipv4/ip_gre.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> > > > > index 22a26d1d29a0..95efa97cb84b 100644
> > > > > --- a/net/ipv4/ip_gre.c
> > > > > +++ b/net/ipv4/ip_gre.c
> > > > > @@ -643,7 +643,8 @@ static netdev_tx_t ipgre_xmit(struct sk_buff *skb,
> > > > >                 /* Pull skb since ip_tunnel_xmit() needs skb->data pointing
> > > > >                  * to gre header.
> > > > >                  */
> > > > > -               skb_pull(skb, tunnel->hlen + sizeof(struct iphdr));
> > > > > +               if (!skb_pull(skb, tunnel->hlen + sizeof(struct iphdr)))
> > > > > +                       goto free_skb;
> > > > >                 skb_reset_mac_header(skb);
> > > > >
> > > > >                 if (skb->ip_summed == CHECKSUM_PARTIAL &&
> > > > > --
> > > >
> > > >
> > > > I have syszbot reports with an actual repro for this one.
> > >
> > > Could you please share them? I could not find easily the reports in
> > > https://syzkaller.appspot.com/upstream
> >
> > Stack trace looks like the following:
> >
> > skbuff: skb_under_panic: text:ffffffff845f50a0 len:920 put:20
> > head:ffff888171931000 data:ffff888171930ff8 tail:0x390 end:0x680
> > dev:gre4
> > ------------[ cut here ]------------
> > kernel BUG at net/core/skbuff.c:120 !
> > invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> > CPU: 0 PID: 12705 Comm: kworker/0:0 Not tainted
> > 6.1.43-syzkaller-00022-g8f46c3493178 #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine,
> > BIOS Google 10/09/2023
> > Workqueue: mld mld_ifc_work
> > RIP: 0010:skb_panic net/core/skbuff.c:120 [inline]
> > RIP: 0010:skb_under_panic+0x14c/0x150 net/core/skbuff.c:130
> > Code: 60 98 da 85 48 c7 c6 05 6b 2f 86 48 8b 55 c0 8b 4d d4 44 8b 45
> > d0 4c 8b 4d c8 53 41 55 41 54 41 57 e8 fc db f4 00 48 83 c4 20 <0f> 0b
> > 66 90 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 18 41
> > RSP: 0018:ffffc9000551f0a0 EFLAGS: 00010286
> > RAX: 0000000000000087 RBX: ffff888162226000 RCX: 98ecdd4da3f28000
> > RDX: 0000000000000000 RSI: 0000000000000400 RDI: 0000000000000000
> > RBP: ffffc9000551f0e0 R08: ffffffff815a9ea5 R09: fffff52000aa3dad
> > R10: 0000000000000000 R11: dffffc0000000001 R12: 0000000000000390
> > R13: 0000000000000680 R14: dffffc0000000000 R15: ffff888171930ff8
> > FS: 0000000000000000(0000) GS:ffff8881f7000000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000001b2f421000 CR3: 000000010f5a3000 CR4: 00000000003506b0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > <TASK>
> > skb_push+0xf3/0x120 net/core/skbuff.c:2181
> > iptunnel_xmit+0x2d0/0x940 net/ipv4/ip_tunnel_core.c:67
> > ip_tunnel_xmit+0x218f/0x2ae0 net/ipv4/ip_tunnel.c:813
> > __gre_xmit net/ipv4/ip_gre.c:469 [inline]
> > ipgre_xmit+0x7ac/0xaa0 net/ipv4/ip_gre.c:661
> > __netdev_start_xmit include/linux/netdevice.h:4908 [inline]
> > netdev_start_xmit include/linux/netdevice.h:4922 [inline]
> > xmit_one net/core/dev.c:3602 [inline]
> > dev_hard_start_xmit+0x1de/0x630 net/core/dev.c:3618
> > __dev_queue_xmit+0x18c0/0x3700 net/core/dev.c:4268
> > dev_queue_xmit include/linux/netdevice.h:3076 [inline]
> > neigh_direct_output+0x17/0x20 net/core/neighbour.c:1592
> > neigh_output include/net/neighbour.h:552 [inline]
> > ip6_finish_output2+0x104a/0x1820 net/ipv6/ip6_output.c:134
> > __ip6_finish_output net/ipv6/ip6_output.c:195 [inline]
> > ip6_finish_output+0x5d9/0xb60 net/ipv6/ip6_output.c:206
> > NF_HOOK_COND include/linux/netfilter.h:294 [inline]
> > ip6_output+0x1f7/0x4d0 net/ipv6/ip6_output.c:227
> > dst_output include/net/dst.h:444 [inline]
> > NF_HOOK include/linux/netfilter.h:305 [inline]
> > mld_sendpack+0x803/0xe40 net/ipv6/mcast.c:1820
> > mld_send_cr net/ipv6/mcast.c:2121 [inline]
> > mld_ifc_work+0x7dc/0xba0 net/ipv6/mcast.c:2653
> > process_one_work+0x73d/0xcb0 kernel/workqueue.c:2299
> > worker_thread+0xa60/0x1260 kernel/workqueue.c:2446
> > kthread+0x26d/0x300 kernel/kthread.c:376
> > ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:306
> > </TASK>
> > Modules linked in:
> > ---[ end trace 0000000000000000 ]---
> > RIP: 0010:skb_panic net/core/skbuff.c:120 [inline]
> > RIP: 0010:skb_under_panic+0x14c/0x150 net/core/skbuff.c:130
> > Code: 60 98 da 85 48 c7 c6 05 6b 2f 86 48 8b 55 c0 8b 4d d4 44 8b 45
> > d0 4c 8b 4d c8 53 41 55 41 54 41 57 e8 fc db f4 00 48 83 c4 20 <0f> 0b
> > 66 90 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 83 ec 18 41
> > RSP: 0018:ffffc9000551f0a0 EFLAGS: 00010286
> > RAX: 0000000000000087 RBX: ffff888162226000 RCX: 98ecdd4da3f28000
> > RDX: 0000000000000000 RSI: 0000000000000400 RDI: 0000000000000000
> > RBP: ffffc9000551f0e0 R08: ffffffff815a9ea5 R09: fffff52000aa3dad
> > R10: 0000000000000000 R11: dffffc0000000001 R12: 0000000000000390
> > R13: 0000000000000680 R14: dffffc0000000000 R15: ffff888171930ff8
> > FS: 0000000000000000(0000) GS:ffff8881f7000000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000001b2f421000 CR3: 000000010f5a3000 CR4: 00000000003506b0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>
> It looks like the repro I had was fixed by "#syz fix: bonding: stop
> the device in bond_setup_by_slave()"
>
> (I am not sure, I have to double check)

This is the syzbot link :
https://syzkaller.appspot.com/bug?extid=802070ddd12342f07fce


>
> # See https://goo.gl/kgGztJ for information about syzkaller reproducers.
> #{"repeat":true,"procs":1,"slowdown":1,"sandbox":"","sandbox_arg":0,"close_fds":false}
> socket$inet(0x2, 0x2, 0x0)
> r0 = socket(0x200000000000011, 0x4000000000080002, 0x0)
> r1 = socket$netlink(0x10, 0x3, 0x0)
> r2 = socket$netlink(0x10, 0x3, 0x0)
> r3 = socket(0x10, 0x803, 0x0)
> sendmsg$NL80211_CMD_CRIT_PROTOCOL_START(r3, &(0x7f0000000580)={0x0,
> 0x0, &(0x7f0000000540)={0x0, 0x1c}}, 0x0)
> getsockname$packet(r3, &(0x7f0000000600)={0x11, 0x0, <r4=>0x0, 0x1,
> 0x0, 0x6, @broadcast}, &(0x7f0000000080)=0x14)
> sendmsg$nl_route(r2, &(0x7f0000000040)={0x0, 0x0,
> &(0x7f0000000000)={&(0x7f0000000340)=ANY=[@ANYBLOB="3c0000001000850600002000fe612233ca000800",
> @ANYRES32=r4, @ANYBLOB="2377f29e252155b21c0012000c000100626f6e64000000000c000200080001000134e7307075a7cc6d2dba6e4dce25f18968dd3d6f77199cd06d7a4cfcdc99dcfd5ec3f3e3d98be8a8bac2dcc414b58dda48b3ea35411d5b112c26f31b352982f55be446b3dd47e435954252213828ba98a1bc363278f8bd13ad746bb8edad619162f5d1892e9fa42e4fe2b60f5fe2bb963f08d6696820ade9cff2b2deb91ce5657168a90dc5230e33b8c26cd925c31366a2ae339f12ba8966be1439cec635b08c0a97490b133a5b7360b59347833fc95a7bf3dc9bc64741de1a6e83c9bdfdfd0baabec981099bb3dbd64a7e7979cfb7935affbcda49190b7ec9bc1e89d6ccedec20f91b571e6fc049ba82821b26ca4f85f4b03f70b176b43de915bec76e405bce49a4b46ec745b51f36282916b77d7f913a6afd6813df2c"],
> 0x3c}}, 0x0)
> sendmsg$nl_route(r1, &(0x7f0000000240)={0x0, 0x0,
> &(0x7f0000000180)={&(0x7f0000000780)=@...link={0x58, 0x10, 0xffffff1f,
> 0x0, 0x0, {0x0, 0x0, 0x0, 0x0, 0x800}, [@IFLA_LINKINFO={0x28, 0x12,
> 0x0, 0x1, @gre={{0x8}, {0x1c, 0x2, 0x0, 0x1, [@IFLA_GRE_LOCAL={0x8,
> 0x6, @broadcast}, @IFLA_GRE_TOS={0x5, 0x9, 0x8}, @IFLA_GRE_OKEY={0x8,
> 0x5, 0x8}]}}}, @IFLA_MASTER={0x8, 0xa, r4}, @IFLA_GROUP={0x8, 0x1b,
> 0x8000}]}, 0x58}}, 0x4004000)
> bind$packet(r0, &(0x7f00000000c0)={0x11, 0x0, r4, 0x1, 0x0, 0x6, @remote}, 0x14)
> sendmsg$nl_route(r0, &(0x7f0000000300)={0x0, 0x0, &(0x7f00000002c0)={0x0}}, 0x0)

Powered by blists - more mailing lists