memory leak in mrp_request_join

memory leak in mrp_request_join

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit:    d96d875e Merge tag 'fixes_for_v5.5-rc8' of git://git.kerne..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14cd8185e00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=15478c61c836a72e
dashboard link: https://syzkaller.appspot.com/bug?extid=5cfab121b54dff775399
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14dbe201e00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13e03cf1e00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+5cfab121b54dff775399@syzkaller.appspotmail.com

executing program
BUG: memory leak
unreferenced object 0xffff888120b85c40 (size 64):
  comm "syz-executor783", pid 7302, jiffies 4294942613 (age 14.030s)
  hex dump (first 32 bytes):
    01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 01 00 00 00 01 02 00 00  ................
  backtrace:
    [<000000000312cb0f>] kmemleak_alloc_recursive include/linux/kmemleak.h:43 [inline]
    [<000000000312cb0f>] slab_post_alloc_hook mm/slab.h:586 [inline]
    [<000000000312cb0f>] slab_alloc mm/slab.c:3320 [inline]
    [<000000000312cb0f>] __do_kmalloc mm/slab.c:3654 [inline]
    [<000000000312cb0f>] __kmalloc+0x169/0x300 mm/slab.c:3665
    [<0000000011736472>] kmalloc include/linux/slab.h:561 [inline]
    [<0000000011736472>] mrp_attr_create net/802/mrp.c:276 [inline]
    [<0000000011736472>] mrp_request_join+0x13d/0x220 net/802/mrp.c:530
    [<000000003ff9bf8f>] vlan_mvrp_request_join+0x86/0x90 net/8021q/vlan_mvrp.c:40
    [<0000000020a13a26>] vlan_dev_open+0x154/0x290 net/8021q/vlan_dev.c:293
    [<0000000010d96d5d>] __dev_open+0x109/0x1b0 net/core/dev.c:1431
    [<00000000e9578876>] __dev_change_flags+0x246/0x2c0 net/core/dev.c:8103
    [<000000002c92e0e1>] rtnl_configure_link+0x57/0x100 net/core/rtnetlink.c:2996
    [<000000000504898e>] __rtnl_newlink+0x8b9/0xb80 net/core/rtnetlink.c:3323
    [<00000000eb645fa3>] rtnl_newlink+0x4e/0x80 net/core/rtnetlink.c:3363
    [<0000000034f28a5d>] rtnetlink_rcv_msg+0x178/0x4b0 net/core/rtnetlink.c:5424
    [<00000000dc499e73>] netlink_rcv_skb+0x61/0x170 net/netlink/af_netlink.c:2477
    [<00000000b6cd4d73>] rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5442
    [<0000000092a30a6a>] netlink_unicast_kernel net/netlink/af_netlink.c:1302 [inline]
    [<0000000092a30a6a>] netlink_unicast+0x223/0x310 net/netlink/af_netlink.c:1328
    [<0000000006ceb85b>] netlink_sendmsg+0x2c0/0x570 net/netlink/af_netlink.c:1917
    [<000000009199ec36>] sock_sendmsg_nosec net/socket.c:639 [inline]
    [<000000009199ec36>] sock_sendmsg+0x54/0x70 net/socket.c:659
    [<000000002acd085e>] ____sys_sendmsg+0x2d0/0x300 net/socket.c:2330



---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

[PATCH] net/802/mrp: disconnect on uninit

[Dan Carpenter]

[  I was investigating this bug and I sort of got carried away and wrote
   a patch.  I'm going to see if I can find a test system to start
   testing these patches then I will resend the patch.  - dan ]

Syzbot discovered that mrp_attr attr structs are being leaked.  They're
supposed to be freed by mrp_attr_destroy() which is called from
mrp_attr_event().

I think that when we close everything down, we're supposed to send one
last disconnect event but the code for that wasn't fully implemented.

Reported-by: syzbot+5cfab121b54dff775399@syzkaller.appspotmail.com
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Not tested.  Idea only.

 net/802/mrp.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/802/mrp.c b/net/802/mrp.c
index bea6e43d45a0..f1d71cd68a79 100644
--- a/net/802/mrp.c
+++ b/net/802/mrp.c
@@ -562,7 +562,9 @@ void mrp_request_leave(const struct net_device *dev,
 }
 EXPORT_SYMBOL_GPL(mrp_request_leave);
 
-static void mrp_mad_event(struct mrp_applicant *app, enum mrp_event event)
+static void mrp_mad_event_helper(struct mrp_applicant *app,
+				 enum mrp_event event,
+				 int state)
 {
 	struct rb_node *node, *next;
 	struct mrp_attr *attr;
@@ -571,10 +573,24 @@ static void mrp_mad_event(struct mrp_applicant *app, enum mrp_event event)
 	     next = node ? rb_next(node) : NULL, node != NULL;
 	     node = next) {
 		attr = rb_entry(node, struct mrp_attr, node);
+		if (state != -1)
+			attr->state = state;
 		mrp_attr_event(app, attr, event);
 	}
 }
 
+static void mrp_mad_event(struct mrp_applicant *app, enum mrp_event event)
+{
+	mrp_mad_event_helper(app, event, -1);
+}
+
+static void mrp_mad_event_state(struct mrp_applicant *app,
+				enum mrp_event event,
+				enum mrp_applicant_state state)
+{
+	mrp_mad_event_helper(app, event, state);
+}
+
 static void mrp_join_timer_arm(struct mrp_applicant *app)
 {
 	unsigned long delay;
@@ -894,7 +910,7 @@ void mrp_uninit_applicant(struct net_device *dev, struct mrp_application *appl)
 	del_timer_sync(&app->periodic_timer);
 
 	spin_lock_bh(&app->lock);
-	mrp_mad_event(app, MRP_EVENT_TX);
+	mrp_mad_event_state(app, MRP_EVENT_TX, MRP_APPLICANT_LA);
 	mrp_pdu_queue(app);
 	spin_unlock_bh(&app->lock);
 
-- 
2.11.0

Re: [PATCH] net/802/mrp: disconnect on uninit

[David Miller]

From: Dan Carpenter <dan.carpenter@oracle.com>
Date: Wed, 22 Jan 2020 10:26:04 +0300

> [  I was investigating this bug and I sort of got carried away and wrote
>    a patch.  I'm going to see if I can find a test system to start
>    testing these patches then I will resend the patch.  - dan ]
> 
> Syzbot discovered that mrp_attr attr structs are being leaked.  They're
> supposed to be freed by mrp_attr_destroy() which is called from
> mrp_attr_event().
> 
> I think that when we close everything down, we're supposed to send one
> last disconnect event but the code for that wasn't fully implemented.
> 
> Reported-by: syzbot+5cfab121b54dff775399@syzkaller.appspotmail.com
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Not tested.  Idea only.

Yes, please resend this when it is tested, somehow, by someone :-)