diff mbox series

[net] ipv6: mcast: better catch silly mtu values

Message ID 1513004618.25033.43.camel@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show
Series [net] ipv6: mcast: better catch silly mtu values | expand

Commit Message

Eric Dumazet Dec. 11, 2017, 3:03 p.m. UTC
From: Eric Dumazet <edumazet@google.com>

syzkaller reported crashes in IPv6 stack [1]

Xin Long found that lo MTU was set to silly values.

IPv6 stack reacts to changes to small MTU, by disabling itself under
RTNL.

But there is a window where threads not using RTNL can see a wrong
device mtu. This can lead to surprises, in mld code where it is assumed
the mtu is suitable.

Fix this by reading device mtu once and checking IPv6 minimal MTU.

[1]
 skbuff: skb_over_panic: text:0000000010b86b8d len:196 put:20
 head:000000003b477e60 data:000000000e85441e tail:0xd4 end:0xc0 dev:lo
 ------------[ cut here ]------------
 kernel BUG at net/core/skbuff.c:104!
 invalid opcode: 0000 [#1] SMP KASAN
 Dumping ftrace buffer:
    (ftrace buffer empty)
 Modules linked in:
 CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.15.0-rc2-mm1+ #39
 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
 Google 01/01/2011
 RIP: 0010:skb_panic+0x15c/0x1f0 net/core/skbuff.c:100
 RSP: 0018:ffff8801db307508 EFLAGS: 00010286
 RAX: 0000000000000082 RBX: ffff8801c517e840 RCX: 0000000000000000
 RDX: 0000000000000082 RSI: 1ffff1003b660e61 RDI: ffffed003b660e95
 RBP: ffff8801db307570 R08: 1ffff1003b660e23 R09: 0000000000000000
 R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff85bd4020
 R13: ffffffff84754ed2 R14: 0000000000000014 R15: ffff8801c4e26540
 FS:  0000000000000000(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
 CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
 CR2: 0000000000463610 CR3: 00000001c6698000 CR4: 00000000001406e0
 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
 Call Trace:
  <IRQ>
  skb_over_panic net/core/skbuff.c:109 [inline]
  skb_put+0x181/0x1c0 net/core/skbuff.c:1694
  add_grhead.isra.24+0x42/0x3b0 net/ipv6/mcast.c:1695
  add_grec+0xa55/0x1060 net/ipv6/mcast.c:1817
  mld_send_cr net/ipv6/mcast.c:1903 [inline]
  mld_ifc_timer_expire+0x4d2/0x770 net/ipv6/mcast.c:2448
  call_timer_fn+0x23b/0x840 kernel/time/timer.c:1320
  expire_timers kernel/time/timer.c:1357 [inline]
  __run_timers+0x7e1/0xb60 kernel/time/timer.c:1660
  run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
  __do_softirq+0x29d/0xbb2 kernel/softirq.c:285
  invoke_softirq kernel/softirq.c:365 [inline]
  irq_exit+0x1d3/0x210 kernel/softirq.c:405
  exiting_irq arch/x86/include/asm/apic.h:540 [inline]
  smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052
  apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:920

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---

Another fix will be sent separately for ipv4 igmp

 net/ipv6/mcast.c |   25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

Comments

Xin Long Dec. 12, 2017, 12:55 p.m. UTC | #1
On Mon, Dec 11, 2017 at 11:03 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller reported crashes in IPv6 stack [1]
>
> Xin Long found that lo MTU was set to silly values.
>
> IPv6 stack reacts to changes to small MTU, by disabling itself under
> RTNL.
>
> But there is a window where threads not using RTNL can see a wrong
> device mtu. This can lead to surprises, in mld code where it is assumed
> the mtu is suitable.
>
> Fix this by reading device mtu once and checking IPv6 minimal MTU.
>
> [1]
>  skbuff: skb_over_panic: text:0000000010b86b8d len:196 put:20
>  head:000000003b477e60 data:000000000e85441e tail:0xd4 end:0xc0 dev:lo
>  ------------[ cut here ]------------
>  kernel BUG at net/core/skbuff.c:104!
>  invalid opcode: 0000 [#1] SMP KASAN
>  Dumping ftrace buffer:
>     (ftrace buffer empty)
>  Modules linked in:
>  CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.15.0-rc2-mm1+ #39
>  Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>  Google 01/01/2011
>  RIP: 0010:skb_panic+0x15c/0x1f0 net/core/skbuff.c:100
>  RSP: 0018:ffff8801db307508 EFLAGS: 00010286
>  RAX: 0000000000000082 RBX: ffff8801c517e840 RCX: 0000000000000000
>  RDX: 0000000000000082 RSI: 1ffff1003b660e61 RDI: ffffed003b660e95
>  RBP: ffff8801db307570 R08: 1ffff1003b660e23 R09: 0000000000000000
>  R10: 0000000000000000 R11: 0000000000000000 R12: ffffffff85bd4020
>  R13: ffffffff84754ed2 R14: 0000000000000014 R15: ffff8801c4e26540
>  FS:  0000000000000000(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
>  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>  CR2: 0000000000463610 CR3: 00000001c6698000 CR4: 00000000001406e0
>  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>  Call Trace:
>   <IRQ>
>   skb_over_panic net/core/skbuff.c:109 [inline]
>   skb_put+0x181/0x1c0 net/core/skbuff.c:1694
>   add_grhead.isra.24+0x42/0x3b0 net/ipv6/mcast.c:1695
>   add_grec+0xa55/0x1060 net/ipv6/mcast.c:1817
>   mld_send_cr net/ipv6/mcast.c:1903 [inline]
>   mld_ifc_timer_expire+0x4d2/0x770 net/ipv6/mcast.c:2448
>   call_timer_fn+0x23b/0x840 kernel/time/timer.c:1320
>   expire_timers kernel/time/timer.c:1357 [inline]
>   __run_timers+0x7e1/0xb60 kernel/time/timer.c:1660
>   run_timer_softirq+0x4c/0xb0 kernel/time/timer.c:1686
>   __do_softirq+0x29d/0xbb2 kernel/softirq.c:285
>   invoke_softirq kernel/softirq.c:365 [inline]
>   irq_exit+0x1d3/0x210 kernel/softirq.c:405
>   exiting_irq arch/x86/include/asm/apic.h:540 [inline]
>   smp_apic_timer_interrupt+0x16b/0x700 arch/x86/kernel/apic/apic.c:1052
>   apic_timer_interrupt+0xa9/0xb0 arch/x86/entry/entry_64.S:920
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Tested-by: Xin Long <lucien.xin@gmail.com>
David Miller Dec. 13, 2017, 6:17 p.m. UTC | #2
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 11 Dec 2017 07:03:38 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> syzkaller reported crashes in IPv6 stack [1]
> 
> Xin Long found that lo MTU was set to silly values.
> 
> IPv6 stack reacts to changes to small MTU, by disabling itself under
> RTNL.
> 
> But there is a window where threads not using RTNL can see a wrong
> device mtu. This can lead to surprises, in mld code where it is assumed
> the mtu is suitable.
> 
> Fix this by reading device mtu once and checking IPv6 minimal MTU.
 ...
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>

Applied and queued up for -stable.
diff mbox series

Patch

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index fc6d7d143f2c29aab9a3f56eae02e5337e65a97b..844642682b8363c4c32d329ed92474f834a59618 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c
@@ -1682,16 +1682,16 @@  static int grec_size(struct ifmcaddr6 *pmc, int type, int gdel, int sdel)
 }
 
 static struct sk_buff *add_grhead(struct sk_buff *skb, struct ifmcaddr6 *pmc,
-	int type, struct mld2_grec **ppgr)
+	int type, struct mld2_grec **ppgr, unsigned int mtu)
 {
-	struct net_device *dev = pmc->idev->dev;
 	struct mld2_report *pmr;
 	struct mld2_grec *pgr;
 
-	if (!skb)
-		skb = mld_newpack(pmc->idev, dev->mtu);
-	if (!skb)
-		return NULL;
+	if (!skb) {
+		skb = mld_newpack(pmc->idev, mtu);
+		if (!skb)
+			return NULL;
+	}
 	pgr = skb_put(skb, sizeof(struct mld2_grec));
 	pgr->grec_type = type;
 	pgr->grec_auxwords = 0;
@@ -1714,10 +1714,15 @@  static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
 	struct mld2_grec *pgr = NULL;
 	struct ip6_sf_list *psf, *psf_next, *psf_prev, **psf_list;
 	int scount, stotal, first, isquery, truncate;
+	unsigned int mtu;
 
 	if (pmc->mca_flags & MAF_NOREPORT)
 		return skb;
 
+	mtu = READ_ONCE(dev->mtu);
+	if (mtu < IPV6_MIN_MTU)
+		return skb;
+
 	isquery = type == MLD2_MODE_IS_INCLUDE ||
 		  type == MLD2_MODE_IS_EXCLUDE;
 	truncate = type == MLD2_MODE_IS_EXCLUDE ||
@@ -1738,7 +1743,7 @@  static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
 		    AVAILABLE(skb) < grec_size(pmc, type, gdeleted, sdeleted)) {
 			if (skb)
 				mld_sendpack(skb);
-			skb = mld_newpack(idev, dev->mtu);
+			skb = mld_newpack(idev, mtu);
 		}
 	}
 	first = 1;
@@ -1774,12 +1779,12 @@  static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
 				pgr->grec_nsrcs = htons(scount);
 			if (skb)
 				mld_sendpack(skb);
-			skb = mld_newpack(idev, dev->mtu);
+			skb = mld_newpack(idev, mtu);
 			first = 1;
 			scount = 0;
 		}
 		if (first) {
-			skb = add_grhead(skb, pmc, type, &pgr);
+			skb = add_grhead(skb, pmc, type, &pgr, mtu);
 			first = 0;
 		}
 		if (!skb)
@@ -1814,7 +1819,7 @@  static struct sk_buff *add_grec(struct sk_buff *skb, struct ifmcaddr6 *pmc,
 				mld_sendpack(skb);
 				skb = NULL; /* add_grhead will get a new one */
 			}
-			skb = add_grhead(skb, pmc, type, &pgr);
+			skb = add_grhead(skb, pmc, type, &pgr, mtu);
 		}
 	}
 	if (pgr)