[Bridge] Use-after-free in br_multicast_rcv

[Bridge] Use-after-free in br_multicast_rcv

[Martin Weinelt]

Hello,

we've recently been experiencing memory leaks on our Linux-based routers,
at least as far back as v4.19.16.

After rebuilding with KASAN it found a use-after-free in 
br_multicast_rcv which I could reproduce on v5.2.0-rc6. 

Please find the KASAN report below, I'm not sure what else to provide so
feel free to ask.

Best,
  Martin


==================================================================
BUG: KASAN: use-after-free in br_multicast_rcv+0x480c/0x4ad0 [bridge]
Read of size 2 at addr ffff8880421302b4 by task ksoftirqd/1/16

CPU: 1 PID: 16 Comm: ksoftirqd/1 Tainted: G           OE     5.2.0-rc6+ #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
Call Trace:
 dump_stack+0x71/0xab
 print_address_description+0x6a/0x280
 ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
 __kasan_report+0x152/0x1aa
 ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
 ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
 kasan_report+0xe/0x20
 br_multicast_rcv+0x480c/0x4ad0 [bridge]
 ? br_multicast_disable_port+0x150/0x150 [bridge]
 ? ktime_get_with_offset+0xb4/0x150
 ? __kasan_kmalloc.constprop.6+0xa6/0xf0
 ? __netif_receive_skb+0x1b0/0x1b0
 ? br_fdb_update+0x10e/0x6e0 [bridge]
 ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
 br_handle_frame_finish+0x3c6/0x11d0 [bridge]
 ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
 ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
 br_handle_frame+0x731/0xd90 [bridge]
 ? select_idle_sibling+0x25/0x7d0
 ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
 __netif_receive_skb_core+0xced/0x2d70
 ? virtqueue_get_buf_ctx+0x230/0x1130 [virtio_ring]
 ? do_xdp_generic+0x20/0x20
 ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
 ? virtnet_poll+0x94d/0xc78 [virtio_net]
 ? receive_buf+0x5120/0x5120 [virtio_net]
 ? __netif_receive_skb_one_core+0x97/0x1d0
 __netif_receive_skb_one_core+0x97/0x1d0
 ? __netif_receive_skb_core+0x2d70/0x2d70
 ? _raw_write_trylock+0x100/0x100
 ? __queue_work+0x41e/0xbe0
 process_backlog+0x19c/0x650
 ? _raw_read_lock_irq+0x40/0x40
 net_rx_action+0x71e/0xbc0
 ? __switch_to_asm+0x40/0x70
 ? napi_complete_done+0x360/0x360
 ? __switch_to_asm+0x34/0x70
 ? __switch_to_asm+0x40/0x70
 ? __schedule+0x85e/0x14d0
 __do_softirq+0x1db/0x5f9
 ? takeover_tasklets+0x5f0/0x5f0
 run_ksoftirqd+0x26/0x40
 smpboot_thread_fn+0x443/0x680
 ? sort_range+0x20/0x20
 ? schedule+0x94/0x210
 ? __kthread_parkme+0x78/0xf0
 ? sort_range+0x20/0x20
 kthread+0x2ae/0x3a0
 ? kthread_create_worker_on_cpu+0xc0/0xc0
 ret_from_fork+0x35/0x40

The buggy address belongs to the page:
page:ffffea0001084c00 refcount:0 mapcount:-128 mapping:0000000000000000 index:0x0
flags: 0xffffc000000000()
raw: 00ffffc000000000 ffffea0000cfca08 ffffea0001098608 0000000000000000
raw: 0000000000000000 0000000000000003 00000000ffffff7f 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff888042130180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff888042130200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>ffff888042130280: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
                                     ^
 ffff888042130300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
 ffff888042130380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
==================================================================
Disabling lock debugging due to kernel taint

Re: [Bridge] Use-after-free in br_multicast_rcv

[nikolay]

On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>Hello,
>
>we've recently been experiencing memory leaks on our Linux-based
>routers,
>at least as far back as v4.19.16.
>
>After rebuilding with KASAN it found a use-after-free in 
>br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>
>Please find the KASAN report below, I'm anot sure what else to provide
>so
>feel free to ask.
>
>Best,
>  Martin
>
>

Hi Martin, 
I'll look into this, are there any specific steps to reproduce it? 

Thanks, 
   Nik

>==================================================================
>BUG: KASAN: use-after-free in br_multicast_rcv+0x480c/0x4ad0 [bridge]
>Read of size 2 at addr ffff8880421302b4 by task ksoftirqd/1/16
>
>CPU: 1 PID: 16 Comm: ksoftirqd/1 Tainted: G           OE     5.2.0-rc6+
>#1
>Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1
>04/01/2014
>Call Trace:
> dump_stack+0x71/0xab
> print_address_description+0x6a/0x280
> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
> __kasan_report+0x152/0x1aa
> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
> kasan_report+0xe/0x20
> br_multicast_rcv+0x480c/0x4ad0 [bridge]
> ? br_multicast_disable_port+0x150/0x150 [bridge]
> ? ktime_get_with_offset+0xb4/0x150
> ? __kasan_kmalloc.constprop.6+0xa6/0xf0
> ? __netif_receive_skb+0x1b0/0x1b0
> ? br_fdb_update+0x10e/0x6e0 [bridge]
> ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
> br_handle_frame_finish+0x3c6/0x11d0 [bridge]
> ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
> ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
> br_handle_frame+0x731/0xd90 [bridge]
> ? select_idle_sibling+0x25/0x7d0
> ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
> __netif_receive_skb_core+0xced/0x2d70
> ? virtqueue_get_buf_ctx+0x230/0x1130 [virtio_ring]
> ? do_xdp_generic+0x20/0x20
> ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
> ? virtnet_poll+0x94d/0xc78 [virtio_net]
> ? receive_buf+0x5120/0x5120 [virtio_net]
> ? __netif_receive_skb_one_core+0x97/0x1d0
> __netif_receive_skb_one_core+0x97/0x1d0
> ? __netif_receive_skb_core+0x2d70/0x2d70
> ? _raw_write_trylock+0x100/0x100
> ? __queue_work+0x41e/0xbe0
> process_backlog+0x19c/0x650
> ? _raw_read_lock_irq+0x40/0x40
> net_rx_action+0x71e/0xbc0
> ? __switch_to_asm+0x40/0x70
> ? napi_complete_done+0x360/0x360
> ? __switch_to_asm+0x34/0x70
> ? __switch_to_asm+0x40/0x70
> ? __schedule+0x85e/0x14d0
> __do_softirq+0x1db/0x5f9
> ? takeover_tasklets+0x5f0/0x5f0
> run_ksoftirqd+0x26/0x40
> smpboot_thread_fn+0x443/0x680
> ? sort_range+0x20/0x20
> ? schedule+0x94/0x210
> ? __kthread_parkme+0x78/0xf0
> ? sort_range+0x20/0x20
> kthread+0x2ae/0x3a0
> ? kthread_create_worker_on_cpu+0xc0/0xc0
> ret_from_fork+0x35/0x40
>
>The buggy address belongs to the page:
>page:ffffea0001084c00 refcount:0 mapcount:-128 mapping:0000000000000000
>index:0x0
>flags: 0xffffc000000000()
>raw: 00ffffc000000000 ffffea0000cfca08 ffffea0001098608
>0000000000000000
>raw: 0000000000000000 0000000000000003 00000000ffffff7f
>0000000000000000
>page dumped because: kasan: bad access detected
>
>Memory state around the buggy address:
> ffff888042130180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff888042130200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>ffff888042130280: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>                                     ^
> ffff888042130300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff888042130380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>==================================================================
>Disabling lock debugging due to kernel taint


-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

Re: [Bridge] Use-after-free in br_multicast_rcv

[Martin Weinelt]

On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>> Hello,
>>
>> we've recently been experiencing memory leaks on our Linux-based
>> routers,
>> at least as far back as v4.19.16.
>>
>> After rebuilding with KASAN it found a use-after-free in 
>> br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>>
>> Please find the KASAN report below, I'm anot sure what else to provide
>> so
>> feel free to ask.
>>
>> Best,
>>  Martin
>>
>>
> 
> Hi Martin, 
> I'll look into this, are there any specific steps to reproduce it? 
> 
> Thanks, 
>    Nik

Hi Nik,

thanks for taking an interest. The setup is a public L2 mesh network
based on the batman-adv kmod. The bridges see 100 Mbps+ of traffic most
of the day and I unfortunately do not yet know what triggers this.


Best,
  Martin

> 
>> ==================================================================
>> BUG: KASAN: use-after-free in br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> Read of size 2 at addr ffff8880421302b4 by task ksoftirqd/1/16
>>
>> CPU: 1 PID: 16 Comm: ksoftirqd/1 Tainted: G           OE     5.2.0-rc6+
>> #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1
>> 04/01/2014
>> Call Trace:
>> dump_stack+0x71/0xab
>> print_address_description+0x6a/0x280
>> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> __kasan_report+0x152/0x1aa
>> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> ? br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> kasan_report+0xe/0x20
>> br_multicast_rcv+0x480c/0x4ad0 [bridge]
>> ? br_multicast_disable_port+0x150/0x150 [bridge]
>> ? ktime_get_with_offset+0xb4/0x150
>> ? __kasan_kmalloc.constprop.6+0xa6/0xf0
>> ? __netif_receive_skb+0x1b0/0x1b0
>> ? br_fdb_update+0x10e/0x6e0 [bridge]
>> ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
>> br_handle_frame_finish+0x3c6/0x11d0 [bridge]
>> ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
>> ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
>> br_handle_frame+0x731/0xd90 [bridge]
>> ? select_idle_sibling+0x25/0x7d0
>> ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
>> __netif_receive_skb_core+0xced/0x2d70
>> ? virtqueue_get_buf_ctx+0x230/0x1130 [virtio_ring]
>> ? do_xdp_generic+0x20/0x20
>> ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
>> ? virtnet_poll+0x94d/0xc78 [virtio_net]
>> ? receive_buf+0x5120/0x5120 [virtio_net]
>> ? __netif_receive_skb_one_core+0x97/0x1d0
>> __netif_receive_skb_one_core+0x97/0x1d0
>> ? __netif_receive_skb_core+0x2d70/0x2d70
>> ? _raw_write_trylock+0x100/0x100
>> ? __queue_work+0x41e/0xbe0
>> process_backlog+0x19c/0x650
>> ? _raw_read_lock_irq+0x40/0x40
>> net_rx_action+0x71e/0xbc0
>> ? __switch_to_asm+0x40/0x70
>> ? napi_complete_done+0x360/0x360
>> ? __switch_to_asm+0x34/0x70
>> ? __switch_to_asm+0x40/0x70
>> ? __schedule+0x85e/0x14d0
>> __do_softirq+0x1db/0x5f9
>> ? takeover_tasklets+0x5f0/0x5f0
>> run_ksoftirqd+0x26/0x40
>> smpboot_thread_fn+0x443/0x680
>> ? sort_range+0x20/0x20
>> ? schedule+0x94/0x210
>> ? __kthread_parkme+0x78/0xf0
>> ? sort_range+0x20/0x20
>> kthread+0x2ae/0x3a0
>> ? kthread_create_worker_on_cpu+0xc0/0xc0
>> ret_from_fork+0x35/0x40
>>
>> The buggy address belongs to the page:
>> page:ffffea0001084c00 refcount:0 mapcount:-128 mapping:0000000000000000
>> index:0x0
>> flags: 0xffffc000000000()
>> raw: 00ffffc000000000 ffffea0000cfca08 ffffea0001098608
>> 0000000000000000
>> raw: 0000000000000000 0000000000000003 00000000ffffff7f
>> 0000000000000000
>> page dumped because: kasan: bad access detected
>>
>> Memory state around the buggy address:
>> ffff888042130180: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ffff888042130200: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>> ffff888042130280: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>>                                     ^
>> ffff888042130300: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ffff888042130380: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
>> ==================================================================
>> Disabling lock debugging due to kernel taint
> 
> 

Re: [Bridge] Use-after-free in br_multicast_rcv

[Martin Weinelt]

Hi Nik,

more info below.

On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>> Hello,
>>
>> we've recently been experiencing memory leaks on our Linux-based
>> routers,
>> at least as far back as v4.19.16.
>>
>> After rebuilding with KASAN it found a use-after-free in 
>> br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>>
>> Please find the KASAN report below, I'm anot sure what else to provide
>> so
>> feel free to ask.
>>
>> Best,
>>  Martin
>>
>>
> 
> Hi Martin, 
> I'll look into this, are there any specific steps to reproduce it? 
> 
> Thanks, 
>    Nik
> 
 
Each server is a KVM Guest and has 18 bridges with the same master/slave
relationships:

  bridge -> batman-adv -> {l2 tunnel, virtio device}

Linus Lüssing from the batman-adv asked me to apply this patch to help
debugging.

v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN 
report, not sure if the additional information at the end is a result of
the added patch though.

Best,
  Martin

From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
Date: Sat, 29 Jun 2019 20:24:23 +0200
Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
 call traces

---
 net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
 1 file changed, 63 insertions(+), 7 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index de22c8fbbb15..224a43318955 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
 					 struct net_bridge_port *port,
 					 const struct in6_addr *group,
 					 __u16 vid, const unsigned char *src);
+
+static noinline void br_ip6_multicast_leave_group_mld2report(
+					 struct net_bridge *br,
+					 struct net_bridge_port *port,
+					 const struct in6_addr *group,
+					 __u16 vid,
+					 const unsigned char *src)
+{
+	br_ip6_multicast_leave_group(br, port, group, vid, src);
+}
+
+static noinline void br_ip6_multicast_leave_group_ipv6rcv(
+					 struct net_bridge *br,
+					 struct net_bridge_port *port,
+					 const struct in6_addr *group,
+					 __u16 vid,
+					 const unsigned char *src)
+{
+	br_ip6_multicast_leave_group(br, port, group, vid, src);
+}
+
+
+static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
+{
+	return ipv6_addr_is_ll_all_nodes(addr);
+}
+
+static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
+{
+	return ipv6_addr_is_ll_all_nodes(addr);
+}
+
+static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
+{
+	return ipv6_addr_is_ll_all_nodes(addr);
+}
 #endif
 
 static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
@@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
 {
 	struct br_ip br_group;
 
-	if (ipv6_addr_is_ll_all_nodes(group))
+	if (ipv6_addr_is_ll_all_nodes_addgroup(group))
 		return 0;
 
 	memset(&br_group, 0, sizeof(br_group));
@@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
 
 	return br_multicast_add_group(br, port, &br_group, src);
 }
+
+static noinline int br_ip6_multicast_add_group_mld2report(
+				      struct net_bridge *br,
+				      struct net_bridge_port *port,
+				      const struct in6_addr *group,
+				      __u16 vid,
+				      const unsigned char *src)
+{
+	return br_ip6_multicast_add_group(br, port, group, vid, src);
+}
+
+static noinline int br_ip6_multicast_add_group_ipv6rcv(
+				      struct net_bridge *br,
+				      struct net_bridge_port *port,
+				      const struct in6_addr *group,
+				      __u16 vid,
+				      const unsigned char *src)
+{
+	return br_ip6_multicast_add_group(br, port, group, vid, src);
+}
 #endif
 
 static void br_multicast_router_expired(struct timer_list *t)
@@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 		if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
 		     grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
 		    ntohs(*nsrcs) == 0) {
-			br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
+			br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
 						     vid, src);
 		} else {
-			err = br_ip6_multicast_add_group(br, port,
+			err = br_ip6_multicast_add_group_mld2report(br, port,
 							 &grec->grec_mca, vid,
 							 src);
 			if (err)
@@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
 	struct br_ip br_group;
 	struct bridge_mcast_own_query *own_query;
 
-	if (ipv6_addr_is_ll_all_nodes(group))
+	if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
 		return;
 
 	own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
@@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	err = ipv6_mc_check_mld(skb);
 
 	if (err == -ENOMSG) {
-		if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
+		if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
 			BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
 
 		if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
@@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	case ICMPV6_MGM_REPORT:
 		src = eth_hdr(skb)->h_source;
 		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
-		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
+		err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
 						 src);
 		break;
 	case ICMPV6_MLD2_REPORT:
@@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 		break;
 	case ICMPV6_MGM_REDUCTION:
 		src = eth_hdr(skb)->h_source;
-		br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
+		br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
 		break;
 	}
 
-- 
2.20.1


[  409.353973] ==================================================================                                                                                                                                     
[  409.356970] BUG: KASAN: out-of-bounds in br_multicast_rcv+0x36e9/0x4080 [bridge]  
[  409.359360] Read of size 2 at addr ffff888046e980b8 by task swapper/2/0      
[  409.361668]                                                                                                                                                                                                                                                                         
[  409.362215] CPU: 2 PID: 0 Comm: swapper/2 Tainted: G           OE     5.2.0-rc6+ #1        
[  409.364503] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[  409.367006] Call Trace:                                                         
[  409.367699]  <IRQ>                                                                                                                                                                                                                                                                  
[  409.368283]  dump_stack+0x71/0xab                                                                                                                                                                                  
[  409.369175]  print_address_description+0x6a/0x280                                  
[  409.370350]  ? br_multicast_rcv+0x36e9/0x4080 [bridge]                                     
[  409.371752]  __kasan_report+0x152/0x1aa                                                                                                                                                                                                                                             
[  409.372703]  ? br_multicast_rcv+0x36e9/0x4080 [bridge]                       
[  409.373950]  ? br_multicast_rcv+0x36e9/0x4080 [bridge]                       
[  409.375308]  kasan_report+0xe/0x20                                           
[  409.376149]  br_multicast_rcv+0x36e9/0x4080 [bridge]                                                                                                                                                                                                                                
[  409.377460]  ? br_multicast_disable_port+0x150/0x150 [bridge]
[  409.379079]  ? kvm_clock_get_cycles+0xd/0x10                          
[  409.380291]  ? __kasan_kmalloc.constprop.6+0xa6/0xf0             
[  409.381693]  ? netif_receive_skb_internal+0x84/0x1a0                                                                                                                                                                                                                                
[  409.383075]  ? __netif_receive_skb+0x1b0/0x1b0      
[  409.384355]  ? br_fdb_update+0x10e/0x6e0 [bridge]                   
[  409.385700]  ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]        
[  409.387283]  br_handle_frame_finish+0x3c6/0x11d0 [bridge]                                                                                                                                                                                                                           
[  409.388807]  ? br_pass_frame_up+0x3a0/0x3a0 [bridge]                   
[  409.390200]  ? virtnet_probe+0x1c80/0x1c80 [virtio_net]               
[  409.391685]  br_handle_frame+0x731/0xd90 [bridge]                
[  409.393017]  ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]                                                                                                                                                                                                                        
[  409.394591]  ? __update_load_avg_se+0x592/0xa70                                   
[  409.395862]  ? __update_load_avg_se+0x592/0xa70                                                                
[  409.397065]  __netif_receive_skb_core+0xced/0x2d70                           
[  409.398208]  ? napi_complete_done+0x10/0x360                                                                                                                                                                                                                                        
[  409.399249]  ? virtqueue_get_buf_ctx+0x271/0x1130 [virtio_ring]                     
[  409.400632]  ? do_xdp_generic+0x20/0x20                                      
[  409.401565]  ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
[  409.404824]  ? virtnet_poll+0x94d/0xc78 [virtio_net]       
[  409.407552]  ? receive_buf+0x5120/0x5120 [virtio_net]                                                                                                                                                              
[  409.410255]  ? __netif_receive_skb_one_core+0x97/0x1d0                            
[  409.413023]  ? account_entity_enqueue+0x340/0x4c0                             
[  409.415639]  __netif_receive_skb_one_core+0x97/0x1d0                                                                                                                                                               
[  409.418346]  ? __netif_receive_skb_core+0x2d70/0x2d70                             
[  409.421143]  ? _raw_write_trylock+0x100/0x100                                 
[  409.423775]  process_backlog+0x19c/0x650                                      
[  409.426383]  net_rx_action+0x71e/0xbc0                                                                                                                                                                             
[  409.428960]  ? napi_complete_done+0x360/0x360                                     
[  409.431693]  ? handle_irq_event_percpu+0xeb/0x140                            
[  409.434705]  ? _raw_spin_lock+0x7a/0xd0                                      
[  409.439881]  ? _raw_write_trylock+0x100/0x100                                
[  409.444967]  __do_softirq+0x1db/0x5f9                                        
[  409.447999]  irq_exit+0x123/0x150                                            
[  409.451116]  do_IRQ+0x71/0x160                                    
[  409.453709]  common_interrupt+0xf/0xf                
[  409.456384]  </IRQ>                                                                                            
[  409.458708] RIP: 0010:native_safe_halt+0xe/0x10                                                                                                                                                                    
[  409.461587] Code: 09 f9 fe 48 8b 04 24 e9 12 ff ff ff e9 07 00 00 00 0f 00 2d d4 60 52 00 f4 c3 66 90 e9 07 00 00 00 0f 00 2d c4 60 52 00 fb f4 <c3> 90 66 66 66 66 90 41 56 41 55 41 54 55 53 e8 7e 05 ba fe 65 44
[  409.469813] RSP: 0018:ffff88805056fd98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd3  
[  409.474002] RAX: ffffffff97f1ad00 RBX: 0000000000000002 RCX: ffffffff96abbbd6
[  409.477775] RDX: 1ffff1100a0a63d0 RSI: 0000000000000004 RDI: ffff8880510b3f38
[  409.481830] RBP: ffffed100a0a63d0 R08: ffffed100a2167e8 R09: ffffed100a2167e7                                  
[  409.486087] R10: ffff8880510b3f3b R11: ffffed100a2167e8 R12: ffffffff98e60480                                                                                                                                      
[  409.490757] R13: 0000000000000002 R14: 0000000000000000 R15: ffff888050531e80       
[  409.495061]  ? ldsem_down_write+0x590/0x590                                         
[  409.499258]  ? rcu_idle_enter+0x106/0x150                                    
[  409.502755]  ? tsc_verify_tsc_adjust+0x96/0x2a0                               
[  409.506287]  default_idle+0x1f/0x280                                          
[  409.509105]  do_idle+0x2d8/0x3e0                                              
[  409.511741]  ? arch_cpu_idle_exit+0x40/0x40                                                                    
[  409.514588]  cpu_startup_entry+0x19/0x20                                      
[  409.517355]  start_secondary+0x316/0x3f0                                            
[  409.520051]  ? set_cpu_sibling_map+0x19c0/0x19c0                                    
[  409.522871]  secondary_startup_64+0xa4/0xb0                                   
[  409.525603]                                                                   
[  409.527708] Allocated by task 0:                        
[  409.530201]  save_stack+0x19/0x70                                             
[  409.532757]  __kasan_kmalloc.constprop.6+0xa6/0xf0                            
[  409.535660]  __kmalloc_node_track_caller+0xe3/0x2c0                           
[  409.538641]  __kmalloc_reserve.isra.45+0x2e/0xb0     
[  409.541426]  pskb_expand_head+0x118/0xcb0                                     
[  409.544107]  __pskb_pull_tail+0xb9/0x1980                                     
[  409.546749]  validate_xmit_skb+0x556/0xab0                                    
[  409.549387]  __dev_queue_xmit+0x11bc/0x2b70                            
[  409.552145]  ip_finish_output2+0xa7e/0x18c0                    
[  409.554817]  ip_output+0x1a1/0x2b0                          
[  409.557335]  ip_forward+0xe39/0x1a20                              
[  409.559875]  ip_rcv+0xb3/0x180                    
[  409.562271]  __netif_receive_skb_one_core+0x145/0x1d0                                                          
[  409.565068]  netif_receive_skb_internal+0x84/0x1a0
[  409.567841]  napi_gro_flush+0x1d7/0x380                                             
[  409.570492]  napi_complete_done+0x172/0x360                                         
[  409.573147]  virtqueue_napi_complete+0x2a/0x70 [virtio_net]
[  409.576154]  virtnet_poll+0x94d/0xc78 [virtio_net]   
[  409.578963]  net_rx_action+0x71e/0xbc0            
[  409.581482]  __do_softirq+0x1db/0x5f9                                         
[  409.584023]                                                                   
[  409.586052] Freed by task 12968:                                              
[  409.588489]  save_stack+0x19/0x70                                 
[  409.591065]  __kasan_slab_free+0x122/0x180                                    
[  409.593775]  kfree+0xa6/0x1f0                                                                                  
[  409.596228]  consume_skb+0x69/0x160                                           
[  409.598803]  tun_do_read+0x53c/0x1b30 [tun]                                         
[  409.601563]  tun_chr_read_iter+0x14a/0x2d0 [tun]                                    
[  409.604477]  new_sync_read+0x3e3/0x6b0                     
[  409.606956]  vfs_read+0xe9/0x2d0
[  409.609283]  ksys_read+0xe8/0x1c0                 
[  409.611566]  do_syscall_64+0xa0/0x2c0                                         
[  409.613927]  entry_SYSCALL_64_after_hwframe+0x44/0xa9                         
[  409.616671]                                                                   
[  409.618573] The buggy address belongs to the object at ffff888046e98000
[  409.618573]  which belongs to the cache kmalloc-2k of size 2048               
[  409.624699] The buggy address is located 184 bytes inside of                  
[  409.624699]  2048-byte region [ffff888046e98000, ffff888046e98800)            
[  409.630602] The buggy address belongs to the page:      
[  409.633445] page:ffffea00011ba600 refcount:1 mapcount:0 mapping:ffff888050c02a80 index:0x0 compound_mapcount: 0
[  409.638069] flags: 0xffffc000010200(slab|head)
[  409.641048] raw: 00ffffc000010200 dead000000000100 dead000000000200 ffff888050c02a80
[  409.644876] raw: 0000000000000000 00000000000f000f 00000001ffffffff 0000000000000000
[  409.648486] page dumped because: kasan: bad access detected
[  409.651569]
[  409.653718] Memory state around the buggy address:
[  409.656635]  ffff888046e97f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[  409.660222]  ffff888046e98000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  409.663775] >ffff888046e98080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  409.667332]                                         ^
[  409.670247]  ffff888046e98100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  409.673815]  ffff888046e98180: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
[  409.677391] ==================================================================
[  409.681002] Disabling lock debugging due to kernel taint

Re: [Bridge] Use-after-free in br_multicast_rcv

[Nikolay Aleksandrov]

Hi Martin,

On 01/07/2019 19:53, Martin Weinelt wrote:
> Hi Nik,
> 
> more info below.
> 
> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>> Hello,
>>>
>>> we've recently been experiencing memory leaks on our Linux-based
>>> routers,
>>> at least as far back as v4.19.16.
>>>
>>> After rebuilding with KASAN it found a use-after-free in 
>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>>>
>>> Please find the KASAN report below, I'm anot sure what else to provide
>>> so
>>> feel free to ask.
>>>
>>> Best,
>>>  Martin
>>>
>>>
>>
>> Hi Martin, 
>> I'll look into this, are there any specific steps to reproduce it? 
>>
>> Thanks, 
>>    Nik
>>>  
> Each server is a KVM Guest and has 18 bridges with the same master/slave
> relationships:
> 
>   bridge -> batman-adv -> {l2 tunnel, virtio device}
> 
> Linus Lüssing from the batman-adv asked me to apply this patch to help
> debugging.
> 
> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN 
> report, not sure if the additional information at the end is a result of
> the added patch though.
> 
> Best,
>   Martin
> 

I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
I've had these on my "to fix" list for some time now, will prepare, test the fixes and
send them for review. In a few minutes I'll send a test patch for you.
That being said, I thought you said you've been experiencing memory leaks, but below
reports are for out-of-bounds accesses, could you please clarify if you were
speaking about these or is there another issue as well ?
If you're experiencing memory leaks, are you sure they're related to the bridge ?
You could try kmemleak for those.

Thank you,
 Nik

> From 47a04e977311a0c45f26905588f563b55239da7f Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Linus=20L=C3=BCssing?= <linus.luessing@c0d3.blue>
> Date: Sat, 29 Jun 2019 20:24:23 +0200
> Subject: [PATCH] bridge: DEBUG: ipv6_addr_is_ll_all_nodes() wrappers for impr.
>  call traces
> 
> ---
>  net/bridge/br_multicast.c | 70 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 63 insertions(+), 7 deletions(-)
> 
> diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> index de22c8fbbb15..224a43318955 100644
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -57,6 +57,42 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
>  					 struct net_bridge_port *port,
>  					 const struct in6_addr *group,
>  					 __u16 vid, const unsigned char *src);
> +
> +static noinline void br_ip6_multicast_leave_group_mld2report(
> +					 struct net_bridge *br,
> +					 struct net_bridge_port *port,
> +					 const struct in6_addr *group,
> +					 __u16 vid,
> +					 const unsigned char *src)
> +{
> +	br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +static noinline void br_ip6_multicast_leave_group_ipv6rcv(
> +					 struct net_bridge *br,
> +					 struct net_bridge_port *port,
> +					 const struct in6_addr *group,
> +					 __u16 vid,
> +					 const unsigned char *src)
> +{
> +	br_ip6_multicast_leave_group(br, port, group, vid, src);
> +}
> +
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_addgroup(const struct in6_addr *addr)
> +{
> +	return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_leavegroup(const struct in6_addr *addr)
> +{
> +	return ipv6_addr_is_ll_all_nodes(addr);
> +}
> +
> +static noinline bool ipv6_addr_is_ll_all_nodes_mcastrcv(const struct in6_addr *addr)
> +{
> +	return ipv6_addr_is_ll_all_nodes(addr);
> +}
>  #endif
>  
>  static struct net_bridge_mdb_entry *br_mdb_ip_get_rcu(struct net_bridge *br,
> @@ -595,7 +631,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
>  {
>  	struct br_ip br_group;
>  
> -	if (ipv6_addr_is_ll_all_nodes(group))
> +	if (ipv6_addr_is_ll_all_nodes_addgroup(group))
>  		return 0;
>  
>  	memset(&br_group, 0, sizeof(br_group));
> @@ -605,6 +641,26 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
>  
>  	return br_multicast_add_group(br, port, &br_group, src);
>  }
> +
> +static noinline int br_ip6_multicast_add_group_mld2report(
> +				      struct net_bridge *br,
> +				      struct net_bridge_port *port,
> +				      const struct in6_addr *group,
> +				      __u16 vid,
> +				      const unsigned char *src)
> +{
> +	return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
> +
> +static noinline int br_ip6_multicast_add_group_ipv6rcv(
> +				      struct net_bridge *br,
> +				      struct net_bridge_port *port,
> +				      const struct in6_addr *group,
> +				      __u16 vid,
> +				      const unsigned char *src)
> +{
> +	return br_ip6_multicast_add_group(br, port, group, vid, src);
> +}
>  #endif
>  
>  static void br_multicast_router_expired(struct timer_list *t)
> @@ -1022,10 +1078,10 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
>  		if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
>  		     grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
>  		    ntohs(*nsrcs) == 0) {
> -			br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
> +			br_ip6_multicast_leave_group_mld2report(br, port, &grec->grec_mca,
>  						     vid, src);
>  		} else {
> -			err = br_ip6_multicast_add_group(br, port,
> +			err = br_ip6_multicast_add_group_mld2report(br, port,
>  							 &grec->grec_mca, vid,
>  							 src);
>  			if (err)
> @@ -1494,7 +1550,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
>  	struct br_ip br_group;
>  	struct bridge_mcast_own_query *own_query;
>  
> -	if (ipv6_addr_is_ll_all_nodes(group))
> +	if (ipv6_addr_is_ll_all_nodes_leavegroup(group))
>  		return;
>  
>  	own_query = port ? &port->ip6_own_query : &br->ip6_own_query;
> @@ -1658,7 +1714,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>  	err = ipv6_mc_check_mld(skb);
>  
>  	if (err == -ENOMSG) {
> -		if (!ipv6_addr_is_ll_all_nodes(&ipv6_hdr(skb)->daddr))
> +		if (!ipv6_addr_is_ll_all_nodes_mcastrcv(&ipv6_hdr(skb)->daddr))
>  			BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
>  
>  		if (ipv6_addr_is_all_snoopers(&ipv6_hdr(skb)->daddr)) {
> @@ -1683,7 +1739,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>  	case ICMPV6_MGM_REPORT:
>  		src = eth_hdr(skb)->h_source;
>  		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
> -		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
> +		err = br_ip6_multicast_add_group_ipv6rcv(br, port, &mld->mld_mca, vid,
>  						 src);
>  		break;
>  	case ICMPV6_MLD2_REPORT:
> @@ -1694,7 +1750,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>  		break;
>  	case ICMPV6_MGM_REDUCTION:
>  		src = eth_hdr(skb)->h_source;
> -		br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
> +		br_ip6_multicast_leave_group_ipv6rcv(br, port, &mld->mld_mca, vid, src);
>  		break;
>  	}
>  
> 

Re: [Bridge] Use-after-free in br_multicast_rcv

[Martin Weinelt]

Hi Nik,

On 7/1/19 7:03 PM, Nikolay Aleksandrov wrote:
> Hi Martin,
> 
> On 01/07/2019 19:53, Martin Weinelt wrote:
>> Hi Nik,
>>
>> more info below.
>>
>> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>>> Hello,
>>>>
>>>> we've recently been experiencing memory leaks on our Linux-based
>>>> routers,
>>>> at least as far back as v4.19.16.
>>>>
>>>> After rebuilding with KASAN it found a use-after-free in 
>>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>>>>
>>>> Please find the KASAN report below, I'm anot sure what else to provide
>>>> so
>>>> feel free to ask.
>>>>
>>>> Best,
>>>>  Martin
>>>>
>>>>
>>>
>>> Hi Martin, 
>>> I'll look into this, are there any specific steps to reproduce it? 
>>>
>>> Thanks, 
>>>    Nik
>>>>  
>> Each server is a KVM Guest and has 18 bridges with the same master/slave
>> relationships:
>>
>>   bridge -> batman-adv -> {l2 tunnel, virtio device}
>>
>> Linus Lüssing from the batman-adv asked me to apply this patch to help
>> debugging.
>>
>> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN 
>> report, not sure if the additional information at the end is a result of
>> the added patch though.
>>
>> Best,
>>   Martin
>>
> 
> I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
> more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
> I've had these on my "to fix" list for some time now, will prepare, test the fixes and
> send them for review. In a few minutes I'll send a test patch for you.
> That being said, I thought you said you've been experiencing memory leaks, but below
> reports are for out-of-bounds accesses, could you please clarify if you were
> speaking about these or is there another issue as well ?
> If you're experiencing memory leaks, are you sure they're related to the bridge ?
> You could try kmemleak for those.
> 
> Thank you,
>  Nik
> 

we had been experiencing memory leaks on v4.19.37, thats why we started to turn on
KASAN and kmemleak in the first place. This is when we found this use-after-free.

The memory leak exists, and is a separate issue. Apparently kmemleak does not work,
I suspect the early log size is too small

root@gw02:~# echo scan > /sys/kernel/debug/kmemleak                                                                                                                                                                                 -bash: echo: write error: Device or resource busy

CONFIG_HAVE_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_KMEMLEAK=y
CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE=400
# CONFIG_DEBUG_KMEMLEAK_TEST is not set
# CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF is not set
CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y

I'll increase the early log size with the next build to try and get more information
on the memory leak, I'll open a separate thread for that then.

Thanks,
  Martin

Re: [Bridge] Use-after-free in br_multicast_rcv

[Nikolay Aleksandrov]

[-- Attachment #1: Type: text/plain, Size: 3413 bytes --]

Hi again,
On 01/07/2019 20:31, Martin Weinelt wrote:
> Hi Nik,
> 
> On 7/1/19 7:03 PM, Nikolay Aleksandrov wrote:
>> Hi Martin,
>>
>> On 01/07/2019 19:53, Martin Weinelt wrote:
>>> Hi Nik,
>>>
>>> more info below.
>>>
>>> On 6/29/19 3:11 PM, nikolay@cumulusnetworks.com wrote:
>>>> On 29 June 2019 14:54:44 EEST, Martin Weinelt <martin@linuxlounge.net> wrote:
>>>>> Hello,
>>>>>
>>>>> we've recently been experiencing memory leaks on our Linux-based
>>>>> routers,
>>>>> at least as far back as v4.19.16.
>>>>>
>>>>> After rebuilding with KASAN it found a use-after-free in 
>>>>> br_multicast_rcv which I could reproduce on v5.2.0-rc6. 
>>>>>
>>>>> Please find the KASAN report below, I'm anot sure what else to provide
>>>>> so
>>>>> feel free to ask.
>>>>>
>>>>> Best,
>>>>>  Martin
>>>>>
>>>>>
>>>>
>>>> Hi Martin, 
>>>> I'll look into this, are there any specific steps to reproduce it? 
>>>>
>>>> Thanks, 
>>>>    Nik
>>>>>  
>>> Each server is a KVM Guest and has 18 bridges with the same master/slave
>>> relationships:
>>>
>>>   bridge -> batman-adv -> {l2 tunnel, virtio device}
>>>
>>> Linus Lüssing from the batman-adv asked me to apply this patch to help
>>> debugging.
>>>
>>> v5.2-rc6-170-g728254541ebc with this patch yielded the following KASAN 
>>> report, not sure if the additional information at the end is a result of
>>> the added patch though.
>>>
>>> Best,
>>>   Martin
>>>
>>
>> I see a couple of issues that can cause out-of-bounds accesses in br_multicast.c
>> more specifically there're pskb_may_pull calls and accesses to stale skb pointers.
>> I've had these on my "to fix" list for some time now, will prepare, test the fixes and
>> send them for review. In a few minutes I'll send a test patch for you.
>> That being said, I thought you said you've been experiencing memory leaks, but below
>> reports are for out-of-bounds accesses, could you please clarify if you were
>> speaking about these or is there another issue as well ?
>> If you're experiencing memory leaks, are you sure they're related to the bridge ?
>> You could try kmemleak for those.
>>
>> Thank you,
>>  Nik
>>
> 
> we had been experiencing memory leaks on v4.19.37, thats why we started to turn on
> KASAN and kmemleak in the first place. This is when we found this use-after-free.
> 
> The memory leak exists, and is a separate issue. Apparently kmemleak does not work,
> I suspect the early log size is too small
> 
> root@gw02:~# echo scan > /sys/kernel/debug/kmemleak                                                                                                                                                                                 -bash: echo: write error: Device or resource busy
> 
> CONFIG_HAVE_DEBUG_KMEMLEAK=y
> CONFIG_DEBUG_KMEMLEAK=y
> CONFIG_DEBUG_KMEMLEAK_EARLY_LOG_SIZE=400
> # CONFIG_DEBUG_KMEMLEAK_TEST is not set
> # CONFIG_DEBUG_KMEMLEAK_DEFAULT_OFF is not set
> CONFIG_DEBUG_KMEMLEAK_AUTO_SCAN=y
> 
> I'll increase the early log size with the next build to try and get more information
> on the memory leak, I'll open a separate thread for that then.
> 
> Thanks,
>   Martin
> 

I see, thanks for clarifying this. So on the KASAN could you please try the attached patch ?
Also could you please run the br_multicast_rcv+xxx addresses through
linux/scripts/faddr2line for your kernel/bridge:
usage: faddr2line [--list] <object file> <func+offset> <func+offset>...

Thanks,
 Nik

[-- Attachment #2: 0001-net-bridge-mcast-fix-possible-uses-of-stale-pointers.patch --]
[-- Type: text/x-patch, Size: 3442 bytes --]

From 5358f2ad1228967d5e8a2dc21e0025651726a3b8 Mon Sep 17 00:00:00 2001
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Mon, 1 Jul 2019 20:31:14 +0300
Subject: [PATCH TEST] net: bridge: mcast: fix possible uses of stale pointers

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 net/bridge/br_multicast.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index de22c8fbbb15..fbedef3fa930 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -917,6 +917,8 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 	len = skb_transport_offset(skb) + sizeof(*ih);
 
 	for (i = 0; i < num; i++) {
+		u16 nsrcs;
+
 		len += sizeof(*grec);
 		if (!ip_mc_may_pull(skb, len))
 			return -EINVAL;
@@ -924,8 +926,9 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 		grec = (void *)(skb->data + len - sizeof(*grec));
 		group = grec->grec_mca;
 		type = grec->grec_type;
+		nsrcs = ntohs(grec->grec_nsrcs);
 
-		len += ntohs(grec->grec_nsrcs) * 4;
+		len += nsrcs * 4;
 		if (!ip_mc_may_pull(skb, len))
 			return -EINVAL;
 
@@ -946,7 +949,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 		src = eth_hdr(skb)->h_source;
 		if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
 		     type == IGMPV3_MODE_IS_INCLUDE) &&
-		    ntohs(grec->grec_nsrcs) == 0) {
+		    nsrcs == 0) {
 			br_ip4_multicast_leave_group(br, port, group, vid, src);
 		} else {
 			err = br_ip4_multicast_add_group(br, port, group, vid,
@@ -983,7 +986,8 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 	len = skb_transport_offset(skb) + sizeof(*icmp6h);
 
 	for (i = 0; i < num; i++) {
-		__be16 *nsrcs, _nsrcs;
+		__be16 *_nsrcs, __nsrcs;
+		u16 nsrcs;
 
 		nsrcs_offset = len + offsetof(struct mld2_grec, grec_nsrcs);
 
@@ -991,12 +995,13 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 		    nsrcs_offset + sizeof(_nsrcs))
 			return -EINVAL;
 
-		nsrcs = skb_header_pointer(skb, nsrcs_offset,
-					   sizeof(_nsrcs), &_nsrcs);
-		if (!nsrcs)
+		_nsrcs = skb_header_pointer(skb, nsrcs_offset,
+					   sizeof(_nsrcs), &__nsrcs);
+		if (!_nsrcs)
 			return -EINVAL;
 
-		grec_len = struct_size(grec, grec_src, ntohs(*nsrcs));
+		nsrcs = ntohs(*_nsrcs);
+		grec_len = struct_size(grec, grec_src, nsrcs);
 
 		if (!ipv6_mc_may_pull(skb, len + grec_len))
 			return -EINVAL;
@@ -1021,7 +1026,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 		src = eth_hdr(skb)->h_source;
 		if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
 		     grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
-		    ntohs(*nsrcs) == 0) {
+		    nsrcs == 0) {
 			br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
 						     vid, src);
 		} else {
@@ -1275,7 +1280,6 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 				  u16 vid)
 {
 	unsigned int transport_len = ipv6_transport_len(skb);
-	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
 	struct mld_msg *mld;
 	struct net_bridge_mdb_entry *mp;
 	struct mld2_query *mld2q;
@@ -1319,7 +1323,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 
 	if (is_general_query) {
 		saddr.proto = htons(ETH_P_IPV6);
-		saddr.u.ip6 = ip6h->saddr;
+		saddr.u.ip6 = ipv6_hdr(skb)->saddr;
 
 		br_multicast_query_received(br, port, &br->ip6_other_query,
 					    &saddr, max_delay);
-- 
2.21.0

Re: [Bridge] Use-after-free in br_multicast_rcv

[Martin Weinelt]

Hi again,

On 7/1/19 7:37 PM, Nikolay Aleksandrov wrote:
> I see, thanks for clarifying this. So on the KASAN could you please try the attached patch ?
> Also could you please run the br_multicast_rcv+xxx addresses through
> linux/scripts/faddr2line for your kernel/bridge:
> usage: faddr2line [--list] <object file> <func+offset> <func+offset>...
> 
> Thanks,
>  Nik
> 

back with a new report. This is 5.2.0-rc7 + your patch.

Best,
  Martin

$ ./faddr2line /usr/lib/debug/lib/modules/5.2.0-rc7+/kernel/net/bridge/bridge.ko br_multicast_rcv+0x4d0/0x4b00
br_multicast_rcv+0x4d0/0x4b00:
__skb_header_pointer at /home/hexa/git/linux/./include/linux/skbuff.h:3476
(inlined by) skb_header_pointer at /home/hexa/git/linux/./include/linux/skbuff.h:3486
(inlined by) br_ip6_multicast_mld2_report at /home/hexa/git/linux/net/bridge/br_multicast.c:998
(inlined by) br_multicast_ipv6_rcv at /home/hexa/git/linux/net/bridge/br_multicast.c:1694
(inlined by) br_multicast_rcv at /home/hexa/git/linux/net/bridge/br_multicast.c:1729


[  329.723036] ==================================================================
[  329.732244] BUG: KASAN: stack-out-of-bounds in skb_copy_bits+0x33e/0x730
[  329.738974] Write of size 8 at addr ffff888050f09860 by task swapper/4/0
[  329.745754]
[  329.749528] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G           OE     5.2.0-rc7+ #2
[  329.756304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[  329.764062] Call Trace:
[  329.768281]  <IRQ>
[  329.772037]  dump_stack+0x71/0xab
[  329.776015]  print_address_description+0x6a/0x280
[  329.780840]  ? skb_copy_bits+0x33e/0x730
[  329.784817]  __kasan_report+0x152/0x1aa
[  329.788623]  ? skb_copy_bits+0x33e/0x730
[  329.792398]  ? skb_copy_bits+0x33e/0x730
[  329.796231]  kasan_report+0xe/0x20
[  329.800250]  memcpy+0x34/0x50
[  329.803716]  skb_copy_bits+0x33e/0x730
[  329.807736]  br_multicast_rcv+0x4d0/0x4b00 [bridge]
[  329.811579]  ? netif_receive_skb_internal+0x84/0x1a0
[  329.815197]  ? br_multicast_disable_port+0x150/0x150 [bridge]
[  329.819164]  ? netif_receive_skb+0x1b/0x1e0
[  329.823374]  ? br_pass_frame_up+0x25b/0x3a0 [bridge]
[  329.828084]  ? br_handle_local_finish+0x20/0x20 [bridge]
[  329.832960]  ? br_fdb_update+0x10e/0x6e0 [bridge]
[  329.837599]  ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
[  329.843090]  br_handle_frame_finish+0x3c6/0x11d0 [bridge]
[  329.848091]  ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
[  329.853063]  ? _raw_write_trylock+0x100/0x100
[  329.857660]  ? update_load_avg+0x1c4/0x1890
[  329.861863]  ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
[  329.866355]  br_handle_frame+0x731/0xd90 [bridge]
[  329.870343]  ? rcu_irq_exit+0x72/0x1c0
[  329.873708]  ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
[  329.878333]  ? do_IRQ+0x71/0x160
[  329.881667]  ? __update_load_avg_cfs_rq+0x2aa/0x980
[  329.885859]  ? common_interrupt+0xa/0xf
[  329.889588]  ? __update_load_avg_cfs_rq+0x2aa/0x980
[  329.894482]  __netif_receive_skb_core+0xced/0x2d70
[  329.900915]  ? napi_complete_done+0x10/0x360
[  329.905743]  ? virtqueue_get_buf_ctx+0x271/0x1130 [virtio_ring]
[  329.909920]  ? do_xdp_generic+0x20/0x20
[  329.912854]  ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
[  329.916242]  ? virtnet_poll+0x94d/0xc78 [virtio_net]
[  329.919606]  ? receive_buf+0x5120/0x5120 [virtio_net]
[  329.924068]  ? __netif_receive_skb_one_core+0x97/0x1d0
[  329.929248]  ? account_entity_enqueue+0x340/0x4c0
[  329.933515]  __netif_receive_skb_one_core+0x97/0x1d0
[  329.937323]  ? __netif_receive_skb_core+0x2d70/0x2d70
[  329.941076]  ? _raw_write_trylock+0x100/0x100
[  329.944515]  process_backlog+0x19c/0x650
[  329.947618]  ? update_cfs_group+0x10b/0x380
[  329.950863]  net_rx_action+0x71e/0xbc0
[  329.953899]  ? napi_complete_done+0x360/0x360
[  329.957240]  ? handle_irq_event_percpu+0xeb/0x140
[  329.960485]  ? _raw_spin_lock+0x7a/0xd0
[  329.962980]  ? _raw_write_trylock+0x100/0x100
[  329.965598]  __do_softirq+0x1db/0x5f9
[  329.968022]  irq_exit+0x123/0x150
[  329.970338]  do_IRQ+0x71/0x160
[  329.972599]  common_interrupt+0xf/0xf
[  329.975034]  </IRQ>
[  329.977167] RIP: 0010:native_safe_halt+0xe/0x10
[  329.979783] Code: 09 f9 fe 48 8b 04 24 e9 12 ff ff ff e9 07 00 00 00 0f 00 2d d4 60 52 00 f4 c3 66 90 e9 07 00 00 00 0f 00 2d c4 60 52 00 fb f4 <c3> 90 66 66 66 66 90 41 56 41 55 41 54 55 53 e8 7e 05 ba fe 65 44
[  329.987611] RSP: 0018:ffff888050347d98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd1
[  329.991311] RAX: ffffffffa3d1ad00 RBX: 0000000000000004 RCX: ffffffffa28bbbd6
[  329.994903] RDX: 1ffff1100a05c5b8 RSI: 0000000000000004 RDI: ffff888050f33f38
[  329.998432] RBP: ffffed100a05c5b8 R08: ffffed100a1e67e8 R09: ffffed100a1e67e7
[  330.002025] R10: ffff888050f33f3b R11: ffffed100a1e67e8 R12: ffffffffa4c604c0
[  330.005736] R13: 0000000000000004 R14: 0000000000000000 R15: ffff8880502e2dc0
[  330.010074]  ? ldsem_down_write+0x590/0x590
[  330.012905]  ? rcu_idle_enter+0x106/0x150
[  330.016130]  ? tsc_verify_tsc_adjust+0x96/0x2a0
[  330.019068]  default_idle+0x1f/0x280
[  330.021710]  do_idle+0x2d8/0x3e0
[  330.024246]  ? arch_cpu_idle_exit+0x40/0x40
[  330.027089]  cpu_startup_entry+0x19/0x20
[  330.030120]  start_secondary+0x316/0x3f0
[  330.032770]  ? set_cpu_sibling_map+0x19c0/0x19c0
[  330.035625]  secondary_startup_64+0xa4/0xb0
[  330.038485]
[  330.040559] The buggy address belongs to the page:
[  330.043525] page:ffffea000143c240 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0
[  330.047223] flags: 0xffffc000001000(reserved)
[  330.050063] raw: 00ffffc000001000 ffffea000143c248 ffffea000143c248 0000000000000000
[  330.053806] raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
[  330.057765] page dumped because: kasan: bad access detected
[  330.060927]
[  330.063115] Memory state around the buggy address:
[  330.066036]  ffff888050f09700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  330.069509]  ffff888050f09780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  330.073066] >ffff888050f09800: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 02 f4 f4 f4
[  330.076586]                                                        ^
[  330.079861]  ffff888050f09880: f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 00 04 f4
[  330.083397]  ffff888050f09900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[  330.086923] ==================================================================
[  330.090465] Disabling lock debugging due to kernel taint

Re: [Bridge] Use-after-free in br_multicast_rcv

[Nikolay Aleksandrov]

[-- Attachment #1: Type: text/plain, Size: 6979 bytes --]

On 7/2/19 1:17 AM, Martin Weinelt wrote:
> Hi again,
> 
> On 7/1/19 7:37 PM, Nikolay Aleksandrov wrote:
>> I see, thanks for clarifying this. So on the KASAN could you please try the attached patch ?
>> Also could you please run the br_multicast_rcv+xxx addresses through
>> linux/scripts/faddr2line for your kernel/bridge:
>> usage: faddr2line [--list] <object file> <func+offset> <func+offset>...
>>
>> Thanks,
>>  Nik
>>
> 
> back with a new report. This is 5.2.0-rc7 + your patch.
> 
> Best,
>   Martin
> 

Thanks! Aaargh.. I made a stupid mistake hurrying to send the patch, apologies.
Here's the fixed version, please give it a go. This report is because
of my change, not because of the previous bug that should've been fixed.


> $ ./faddr2line /usr/lib/debug/lib/modules/5.2.0-rc7+/kernel/net/bridge/bridge.ko br_multicast_rcv+0x4d0/0x4b00
> br_multicast_rcv+0x4d0/0x4b00:
> __skb_header_pointer at /home/hexa/git/linux/./include/linux/skbuff.h:3476
> (inlined by) skb_header_pointer at /home/hexa/git/linux/./include/linux/skbuff.h:3486
> (inlined by) br_ip6_multicast_mld2_report at /home/hexa/git/linux/net/bridge/br_multicast.c:998
> (inlined by) br_multicast_ipv6_rcv at /home/hexa/git/linux/net/bridge/br_multicast.c:1694
> (inlined by) br_multicast_rcv at /home/hexa/git/linux/net/bridge/br_multicast.c:1729
> 
> 
> [  329.723036] ==================================================================
> [  329.732244] BUG: KASAN: stack-out-of-bounds in skb_copy_bits+0x33e/0x730
> [  329.738974] Write of size 8 at addr ffff888050f09860 by task swapper/4/0
> [  329.745754]
> [  329.749528] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G           OE     5.2.0-rc7+ #2
> [  329.756304] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> [  329.764062] Call Trace:
> [  329.768281]  <IRQ>
> [  329.772037]  dump_stack+0x71/0xab
> [  329.776015]  print_address_description+0x6a/0x280
> [  329.780840]  ? skb_copy_bits+0x33e/0x730
> [  329.784817]  __kasan_report+0x152/0x1aa
> [  329.788623]  ? skb_copy_bits+0x33e/0x730
> [  329.792398]  ? skb_copy_bits+0x33e/0x730
> [  329.796231]  kasan_report+0xe/0x20
> [  329.800250]  memcpy+0x34/0x50
> [  329.803716]  skb_copy_bits+0x33e/0x730
> [  329.807736]  br_multicast_rcv+0x4d0/0x4b00 [bridge]
> [  329.811579]  ? netif_receive_skb_internal+0x84/0x1a0
> [  329.815197]  ? br_multicast_disable_port+0x150/0x150 [bridge]
> [  329.819164]  ? netif_receive_skb+0x1b/0x1e0
> [  329.823374]  ? br_pass_frame_up+0x25b/0x3a0 [bridge]
> [  329.828084]  ? br_handle_local_finish+0x20/0x20 [bridge]
> [  329.832960]  ? br_fdb_update+0x10e/0x6e0 [bridge]
> [  329.837599]  ? br_handle_frame_finish+0x3c6/0x11d0 [bridge]
> [  329.843090]  br_handle_frame_finish+0x3c6/0x11d0 [bridge]
> [  329.848091]  ? br_pass_frame_up+0x3a0/0x3a0 [bridge]
> [  329.853063]  ? _raw_write_trylock+0x100/0x100
> [  329.857660]  ? update_load_avg+0x1c4/0x1890
> [  329.861863]  ? virtnet_probe+0x1c80/0x1c80 [virtio_net]
> [  329.866355]  br_handle_frame+0x731/0xd90 [bridge]
> [  329.870343]  ? rcu_irq_exit+0x72/0x1c0
> [  329.873708]  ? br_handle_frame_finish+0x11d0/0x11d0 [bridge]
> [  329.878333]  ? do_IRQ+0x71/0x160
> [  329.881667]  ? __update_load_avg_cfs_rq+0x2aa/0x980
> [  329.885859]  ? common_interrupt+0xa/0xf
> [  329.889588]  ? __update_load_avg_cfs_rq+0x2aa/0x980
> [  329.894482]  __netif_receive_skb_core+0xced/0x2d70
> [  329.900915]  ? napi_complete_done+0x10/0x360
> [  329.905743]  ? virtqueue_get_buf_ctx+0x271/0x1130 [virtio_ring]
> [  329.909920]  ? do_xdp_generic+0x20/0x20
> [  329.912854]  ? virtqueue_napi_complete+0x39/0x70 [virtio_net]
> [  329.916242]  ? virtnet_poll+0x94d/0xc78 [virtio_net]
> [  329.919606]  ? receive_buf+0x5120/0x5120 [virtio_net]
> [  329.924068]  ? __netif_receive_skb_one_core+0x97/0x1d0
> [  329.929248]  ? account_entity_enqueue+0x340/0x4c0
> [  329.933515]  __netif_receive_skb_one_core+0x97/0x1d0
> [  329.937323]  ? __netif_receive_skb_core+0x2d70/0x2d70
> [  329.941076]  ? _raw_write_trylock+0x100/0x100
> [  329.944515]  process_backlog+0x19c/0x650
> [  329.947618]  ? update_cfs_group+0x10b/0x380
> [  329.950863]  net_rx_action+0x71e/0xbc0
> [  329.953899]  ? napi_complete_done+0x360/0x360
> [  329.957240]  ? handle_irq_event_percpu+0xeb/0x140
> [  329.960485]  ? _raw_spin_lock+0x7a/0xd0
> [  329.962980]  ? _raw_write_trylock+0x100/0x100
> [  329.965598]  __do_softirq+0x1db/0x5f9
> [  329.968022]  irq_exit+0x123/0x150
> [  329.970338]  do_IRQ+0x71/0x160
> [  329.972599]  common_interrupt+0xf/0xf
> [  329.975034]  </IRQ>
> [  329.977167] RIP: 0010:native_safe_halt+0xe/0x10
> [  329.979783] Code: 09 f9 fe 48 8b 04 24 e9 12 ff ff ff e9 07 00 00 00 0f 00 2d d4 60 52 00 f4 c3 66 90 e9 07 00 00 00 0f 00 2d c4 60 52 00 fb f4 <c3> 90 66 66 66 66 90 41 56 41 55 41 54 55 53 e8 7e 05 ba fe 65 44
> [  329.987611] RSP: 0018:ffff888050347d98 EFLAGS: 00000246 ORIG_RAX: ffffffffffffffd1
> [  329.991311] RAX: ffffffffa3d1ad00 RBX: 0000000000000004 RCX: ffffffffa28bbbd6
> [  329.994903] RDX: 1ffff1100a05c5b8 RSI: 0000000000000004 RDI: ffff888050f33f38
> [  329.998432] RBP: ffffed100a05c5b8 R08: ffffed100a1e67e8 R09: ffffed100a1e67e7
> [  330.002025] R10: ffff888050f33f3b R11: ffffed100a1e67e8 R12: ffffffffa4c604c0
> [  330.005736] R13: 0000000000000004 R14: 0000000000000000 R15: ffff8880502e2dc0
> [  330.010074]  ? ldsem_down_write+0x590/0x590
> [  330.012905]  ? rcu_idle_enter+0x106/0x150
> [  330.016130]  ? tsc_verify_tsc_adjust+0x96/0x2a0
> [  330.019068]  default_idle+0x1f/0x280
> [  330.021710]  do_idle+0x2d8/0x3e0
> [  330.024246]  ? arch_cpu_idle_exit+0x40/0x40
> [  330.027089]  cpu_startup_entry+0x19/0x20
> [  330.030120]  start_secondary+0x316/0x3f0
> [  330.032770]  ? set_cpu_sibling_map+0x19c0/0x19c0
> [  330.035625]  secondary_startup_64+0xa4/0xb0
> [  330.038485]
> [  330.040559] The buggy address belongs to the page:
> [  330.043525] page:ffffea000143c240 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0
> [  330.047223] flags: 0xffffc000001000(reserved)
> [  330.050063] raw: 00ffffc000001000 ffffea000143c248 ffffea000143c248 0000000000000000
> [  330.053806] raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
> [  330.057765] page dumped because: kasan: bad access detected
> [  330.060927]
> [  330.063115] Memory state around the buggy address:
> [  330.066036]  ffff888050f09700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  330.069509]  ffff888050f09780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  330.073066] >ffff888050f09800: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 02 f4 f4 f4
> [  330.076586]                                                        ^
> [  330.079861]  ffff888050f09880: f2 f2 f2 f2 04 f4 f4 f4 f2 f2 f2 f2 00 00 04 f4
> [  330.083397]  ffff888050f09900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> [  330.086923] ==================================================================
> [  330.090465] Disabling lock debugging due to kernel taint
> 


[-- Attachment #2: 0001-net-bridge-mcast-fix-possible-uses-of-stale-pointers.patch --]
[-- Type: text/x-patch, Size: 3465 bytes --]

From c99a71f05ec8643745f435c179c21dda6079c56a Mon Sep 17 00:00:00 2001
From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Date: Mon, 1 Jul 2019 20:31:14 +0300
Subject: [PATCH TEST v2] net: bridge: mcast: fix possible uses of stale pointers

Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 net/bridge/br_multicast.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index de22c8fbbb15..d3bb841942b0 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -917,6 +917,8 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 	len = skb_transport_offset(skb) + sizeof(*ih);
 
 	for (i = 0; i < num; i++) {
+		u16 nsrcs;
+
 		len += sizeof(*grec);
 		if (!ip_mc_may_pull(skb, len))
 			return -EINVAL;
@@ -924,8 +926,9 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 		grec = (void *)(skb->data + len - sizeof(*grec));
 		group = grec->grec_mca;
 		type = grec->grec_type;
+		nsrcs = ntohs(grec->grec_nsrcs);
 
-		len += ntohs(grec->grec_nsrcs) * 4;
+		len += nsrcs * 4;
 		if (!ip_mc_may_pull(skb, len))
 			return -EINVAL;
 
@@ -946,7 +949,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 		src = eth_hdr(skb)->h_source;
 		if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
 		     type == IGMPV3_MODE_IS_INCLUDE) &&
-		    ntohs(grec->grec_nsrcs) == 0) {
+		    nsrcs == 0) {
 			br_ip4_multicast_leave_group(br, port, group, vid, src);
 		} else {
 			err = br_ip4_multicast_add_group(br, port, group, vid,
@@ -983,20 +986,22 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 	len = skb_transport_offset(skb) + sizeof(*icmp6h);
 
 	for (i = 0; i < num; i++) {
-		__be16 *nsrcs, _nsrcs;
+		__be16 *_nsrcs, __nsrcs;
+		u16 nsrcs;
 
 		nsrcs_offset = len + offsetof(struct mld2_grec, grec_nsrcs);
 
 		if (skb_transport_offset(skb) + ipv6_transport_len(skb) <
-		    nsrcs_offset + sizeof(_nsrcs))
+		    nsrcs_offset + sizeof(__nsrcs))
 			return -EINVAL;
 
-		nsrcs = skb_header_pointer(skb, nsrcs_offset,
-					   sizeof(_nsrcs), &_nsrcs);
-		if (!nsrcs)
+		_nsrcs = skb_header_pointer(skb, nsrcs_offset,
+					   sizeof(__nsrcs), &__nsrcs);
+		if (!_nsrcs)
 			return -EINVAL;
 
-		grec_len = struct_size(grec, grec_src, ntohs(*nsrcs));
+		nsrcs = ntohs(*_nsrcs);
+		grec_len = struct_size(grec, grec_src, nsrcs);
 
 		if (!ipv6_mc_may_pull(skb, len + grec_len))
 			return -EINVAL;
@@ -1021,7 +1026,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 		src = eth_hdr(skb)->h_source;
 		if ((grec->grec_type == MLD2_CHANGE_TO_INCLUDE ||
 		     grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
-		    ntohs(*nsrcs) == 0) {
+		    nsrcs == 0) {
 			br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
 						     vid, src);
 		} else {
@@ -1275,7 +1280,6 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 				  u16 vid)
 {
 	unsigned int transport_len = ipv6_transport_len(skb);
-	const struct ipv6hdr *ip6h = ipv6_hdr(skb);
 	struct mld_msg *mld;
 	struct net_bridge_mdb_entry *mp;
 	struct mld2_query *mld2q;
@@ -1319,7 +1323,7 @@ static int br_ip6_multicast_query(struct net_bridge *br,
 
 	if (is_general_query) {
 		saddr.proto = htons(ETH_P_IPV6);
-		saddr.u.ip6 = ip6h->saddr;
+		saddr.u.ip6 = ipv6_hdr(skb)->saddr;
 
 		br_multicast_query_received(br, port, &br->ip6_other_query,
 					    &saddr, max_delay);
-- 
2.21.0

Re: [Bridge] Use-after-free in br_multicast_rcv

[Martin Weinelt]

Hi Nik,

On 7/2/19 12:37 AM, Nikolay Aleksandrov wrote:
> On 7/2/19 1:17 AM, Martin Weinelt wrote:
>> Hi again,
>>
>> On 7/1/19 7:37 PM, Nikolay Aleksandrov wrote:
>>> I see, thanks for clarifying this. So on the KASAN could you please try the attached patch ?
>>> Also could you please run the br_multicast_rcv+xxx addresses through
>>> linux/scripts/faddr2line for your kernel/bridge:
>>> usage: faddr2line [--list] <object file> <func+offset> <func+offset>...
>>>
>>> Thanks,
>>>  Nik
>>>
>>
>> back with a new report. This is 5.2.0-rc7 + your patch.
>>
>> Best,
>>   Martin
>>
> 
> Thanks! Aaargh.. I made a stupid mistake hurrying to send the patch, apologies.
> Here's the fixed version, please give it a go. This report is because
> of my change, not because of the previous bug that should've been fixed.
> 

I applied your latest patch against 5.2.0-rc7 and it seems to have fixed the issue as,
after 6 hours of uptime, the KASAN report isn't coming up anymore.

Also there are currently no kmemleak results coming up on 5.2.0-rc7, so I'll be
looking at the v4.19.x series next.

Thank you!

Best
  Martin

Re: [Bridge] [PATCH] fix noderef.cocci warnings

[Nikolay Aleksandrov]

On 7/24/19 1:54 AM, kbuild test robot wrote:
> From: kbuild test robot <lkp@intel.com>
> 
> net/bridge/br_multicast.c:999:8-14: ERROR: application of sizeof to pointer
> 
>  sizeof when applied to a pointer typed expression gives the size of
>  the pointer
> 
> Generated by: scripts/coccinelle/misc/noderef.cocci
> 
> Fixes: 17c91348ed8b ("Use-after-free in br_multicast_rcv")
> CC: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
> Signed-off-by: kbuild test robot <lkp@intel.com>
> ---
> 
> url:    https://github.com/0day-ci/linux/commits/Nikolay-Aleksandrov/net-bridge-mcast-fix-possible-uses-of-stale-pointers/20190702-083354
> 
>  br_multicast.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/net/bridge/br_multicast.c
> +++ b/net/bridge/br_multicast.c
> @@ -996,7 +996,7 @@ static int br_ip6_multicast_mld2_report(
>  			return -EINVAL;
>  
>  		_nsrcs = skb_header_pointer(skb, nsrcs_offset,
> -					   sizeof(_nsrcs), &__nsrcs);
> +					   sizeof(*_nsrcs), &__nsrcs);
>  		if (!_nsrcs)
>  			return -EINVAL;
>  
> 

This must be quite old, I already sent a proper patch without this error.
This one was sent just for testing, hence the TEST in $subject.

 [PATCH TEST] net: bridge: mcast: fix possible uses of stale pointers

Thanks,
 Nik