memory leak in mcba_usb_probe

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit: 467f8165 Merge tag 'close-range-cloexec-unshare-v5.11' of ..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15933e0f500000
kernel config: https://syzkaller.appspot.com/x/.config?x=37c889fb8b2761af
dashboard link: https://syzkaller.appspot.com/bug?extid=57281c762a3922e14dfe
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14fe2b9b500000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13bd2287500000

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+57281c...@syzkaller.appspotmail.com

BUG: memory leak
unreferenced object 0xffff88811101c080 (size 64):
comm "kworker/0:2", pid 56, jiffies 4294942309 (age 8.240s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e42b7c1c>] kmalloc include/linux/slab.h:557 [inline]
[<00000000e42b7c1c>] hcd_buffer_alloc+0x149/0x190 drivers/usb/core/buffer.c:134
[<0000000042899d1a>] usb_alloc_coherent+0x42/0x60 drivers/usb/core/usb.c:897
[<00000000abbb04b6>] mcba_usb_start drivers/net/can/usb/mcba_usb.c:644 [inline]
[<00000000abbb04b6>] mcba_usb_probe+0x27b/0x430 drivers/net/can/usb/mcba_usb.c:846
[<00000000b5e48b67>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
[<00000000b0116c0e>] __device_attach_driver+0xee/0x110 drivers/base/dd.c:851
[<00000000da4bf16e>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:431
[<00000000e826f962>] __device_attach+0x122/0x250 drivers/base/dd.c:919
[<00000000fb35d32b>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:491
[<0000000083f168a1>] device_add+0x5be/0xc30 drivers/base/core.c:3091
[<000000002b4245b1>] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2164
[<00000000fa502fa0>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238
[<000000004e1ab3a9>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745

BUG: memory leak
unreferenced object 0xffff88811101c0c0 (size 64):
comm "kworker/0:2", pid 56, jiffies 4294942309 (age 8.240s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e42b7c1c>] kmalloc include/linux/slab.h:557 [inline]
[<00000000e42b7c1c>] hcd_buffer_alloc+0x149/0x190 drivers/usb/core/buffer.c:134
[<0000000042899d1a>] usb_alloc_coherent+0x42/0x60 drivers/usb/core/usb.c:897
[<00000000abbb04b6>] mcba_usb_start drivers/net/can/usb/mcba_usb.c:644 [inline]
[<00000000abbb04b6>] mcba_usb_probe+0x27b/0x430 drivers/net/can/usb/mcba_usb.c:846
[<00000000b5e48b67>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
[<00000000b0116c0e>] __device_attach_driver+0xee/0x110 drivers/base/dd.c:851
[<00000000da4bf16e>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:431
[<00000000e826f962>] __device_attach+0x122/0x250 drivers/base/dd.c:919
[<00000000fb35d32b>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:491
[<0000000083f168a1>] device_add+0x5be/0xc30 drivers/base/core.c:3091
[<000000002b4245b1>] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2164
[<00000000fa502fa0>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238
[<000000004e1ab3a9>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745

BUG: memory leak
unreferenced object 0xffff88811101c100 (size 64):
comm "kworker/0:2", pid 56, jiffies 4294942309 (age 8.240s)
hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<00000000e42b7c1c>] kmalloc include/linux/slab.h:557 [inline]
[<00000000e42b7c1c>] hcd_buffer_alloc+0x149/0x190 drivers/usb/core/buffer.c:134
[<0000000042899d1a>] usb_alloc_coherent+0x42/0x60 drivers/usb/core/usb.c:897
[<00000000abbb04b6>] mcba_usb_start drivers/net/can/usb/mcba_usb.c:644 [inline]
[<00000000abbb04b6>] mcba_usb_probe+0x27b/0x430 drivers/net/can/usb/mcba_usb.c:846
[<00000000b5e48b67>] usb_probe_interface+0x177/0x370 drivers/usb/core/driver.c:396
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745
[<00000000b0116c0e>] __device_attach_driver+0xee/0x110 drivers/base/dd.c:851
[<00000000da4bf16e>] bus_for_each_drv+0xb7/0x100 drivers/base/bus.c:431
[<00000000e826f962>] __device_attach+0x122/0x250 drivers/base/dd.c:919
[<00000000fb35d32b>] bus_probe_device+0xc6/0xe0 drivers/base/bus.c:491
[<0000000083f168a1>] device_add+0x5be/0xc30 drivers/base/core.c:3091
[<000000002b4245b1>] usb_set_configuration+0x9d9/0xb90 drivers/usb/core/message.c:2164
[<00000000fa502fa0>] usb_generic_driver_probe+0x8c/0xc0 drivers/usb/core/generic.c:238
[<000000004e1ab3a9>] usb_probe_device+0x5c/0x140 drivers/usb/core/driver.c:293
[<00000000a0416f71>] really_probe+0x159/0x480 drivers/base/dd.c:561
[<00000000cce96f4d>] driver_probe_device+0x84/0x100 drivers/base/dd.c:745



---
This report 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 syzk...@googlegroups.com.

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

[Bui Quang Minh]

#syz test: https://github.com/minhbq-99/linux.git 080e743dff190ee8ebec63a13ac33fe8b7e4fc9e

[Bui Quang Minh]

In mcba_usb_read_bulk_callback(), when we don't resubmit the urb, we
need to deallocate the transfer buffer that is allocated in
mcba_usb_start().

Reported-by: syzbot+57281c...@syzkaller.appspotmail.com
Signed-off-by: Bui Quang Minh <minhqua...@gmail.com>
---
drivers/net/can/usb/mcba_usb.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index df54eb7d4b36..7375c384cbd2 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -584,6 +584,8 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
case -EPIPE:
case -EPROTO:
case -ESHUTDOWN:
+ usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+ urb->transfer_buffer, urb->transfer_dma);
return;

default:
--
2.17.1

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
memory leak in rxrpc_lookup_local

BUG: memory leak
unreferenced object 0xffff888118b2f300 (size 256):
comm "syz-executor.5", pid 8860, jiffies 4294944050 (age 373.640s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 80 c3 18 81 88 ff ff ................
backtrace:
[<00000000799c996d>] kmalloc include/linux/slab.h:552 [inline]
[<00000000799c996d>] kzalloc include/linux/slab.h:682 [inline]
[<00000000799c996d>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<00000000799c996d>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<00000000a9547b72>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<00000000c8d0f9b2>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<0000000029791b25>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005937929d>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<0000000064e0e1db>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<00000000ca4d5a61>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<0000000029feabb4>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<000000005ce44751>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000003c257699>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<000000001788a4be>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<000000001788a4be>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<000000001788a4be>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<00000000db77e81f>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<0000000052c5dd81>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff888118afb200 (size 256):
comm "syz-executor.0", pid 8858, jiffies 4294944054 (age 373.600s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 00 92 18 81 88 ff ff ................
backtrace:
[<00000000799c996d>] kmalloc include/linux/slab.h:552 [inline]
[<00000000799c996d>] kzalloc include/linux/slab.h:682 [inline]
[<00000000799c996d>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<00000000799c996d>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<00000000a9547b72>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<00000000c8d0f9b2>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<0000000029791b25>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005937929d>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<0000000064e0e1db>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<00000000ca4d5a61>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<0000000029feabb4>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<000000005ce44751>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000003c257699>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<000000001788a4be>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<000000001788a4be>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<000000001788a4be>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<00000000db77e81f>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<0000000052c5dd81>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff8881189f4800 (size 256):
comm "syz-executor.6", pid 8856, jiffies 4294944054 (age 373.600s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 c0 56 18 81 88 ff ff ..........V.....
backtrace:
[<00000000799c996d>] kmalloc include/linux/slab.h:552 [inline]
[<00000000799c996d>] kzalloc include/linux/slab.h:682 [inline]
[<00000000799c996d>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<00000000799c996d>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<00000000a9547b72>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<00000000c8d0f9b2>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<0000000029791b25>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005937929d>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<0000000064e0e1db>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<00000000ca4d5a61>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<0000000029feabb4>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<000000005ce44751>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000003c257699>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<000000001788a4be>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<000000001788a4be>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<000000001788a4be>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<00000000db77e81f>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<0000000052c5dd81>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff888118c34100 (size 256):
comm "syz-executor.2", pid 8861, jiffies 4294944055 (age 373.590s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 40 56 18 81 88 ff ff .........@V.....
backtrace:
[<00000000799c996d>] kmalloc include/linux/slab.h:552 [inline]
[<00000000799c996d>] kzalloc include/linux/slab.h:682 [inline]
[<00000000799c996d>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<00000000799c996d>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<00000000a9547b72>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<00000000c8d0f9b2>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<0000000029791b25>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005937929d>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<0000000064e0e1db>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<00000000ca4d5a61>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<0000000029feabb4>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<000000005ce44751>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000003c257699>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<000000001788a4be>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<000000001788a4be>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<000000001788a4be>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<00000000db77e81f>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<0000000052c5dd81>] entry_SYSCALL_64_after_hwframe+0x44/0xa9



Tested on:

commit: 080e743d can: mcba_usb: Fix memory leak when cancelling urb
git tree: https://github.com/minhbq-99/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=1322c7a8d00000
kernel config: https://syzkaller.appspot.com/x/.config?x=67a7baec24faba80

[syzbot]

Hello,

syzbot has tested the proposed patch but the reproducer is still triggering an issue:
memory leak in rxrpc_lookup_local

BUG: memory leak

unreferenced object 0xffff888119374900 (size 256):
comm "syz-executor.4", pid 8875, jiffies 4294944284 (age 373.440s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 c0 27 18 81 88 ff ff ..........'.....
backtrace:
[<000000009ef3a83b>] kmalloc include/linux/slab.h:552 [inline]
[<000000009ef3a83b>] kzalloc include/linux/slab.h:682 [inline]
[<000000009ef3a83b>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<000000009ef3a83b>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<0000000050950ced>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<000000000ac020bd>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<00000000e684bacb>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005fdca026>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<000000009edc1bfd>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<0000000022d18a10>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<00000000775d29a1>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<00000000ea080f0c>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000002c51c8c8>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<00000000177a9d84>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<00000000177a9d84>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<00000000177a9d84>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<000000000e9763c5>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000fd35138e>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88811341e300 (size 256):
comm "syz-executor.3", pid 8879, jiffies 4294944293 (age 373.350s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 00 22 19 81 88 ff ff ..........".....
backtrace:
[<000000009ef3a83b>] kmalloc include/linux/slab.h:552 [inline]
[<000000009ef3a83b>] kzalloc include/linux/slab.h:682 [inline]
[<000000009ef3a83b>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<000000009ef3a83b>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<0000000050950ced>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<000000000ac020bd>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<00000000e684bacb>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005fdca026>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<000000009edc1bfd>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<0000000022d18a10>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<00000000775d29a1>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<00000000ea080f0c>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000002c51c8c8>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<00000000177a9d84>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<00000000177a9d84>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<00000000177a9d84>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<000000000e9763c5>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000fd35138e>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff88811734c900 (size 256):
comm "syz-executor.2", pid 8889, jiffies 4294944321 (age 373.070s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 c0 e6 16 81 88 ff ff ................
backtrace:
[<000000009ef3a83b>] kmalloc include/linux/slab.h:552 [inline]
[<000000009ef3a83b>] kzalloc include/linux/slab.h:682 [inline]
[<000000009ef3a83b>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<000000009ef3a83b>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<0000000050950ced>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<000000000ac020bd>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<00000000e684bacb>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005fdca026>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<000000009edc1bfd>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<0000000022d18a10>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<00000000775d29a1>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<00000000ea080f0c>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000002c51c8c8>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<00000000177a9d84>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<00000000177a9d84>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<00000000177a9d84>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<000000000e9763c5>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000fd35138e>] entry_SYSCALL_64_after_hwframe+0x44/0xa9

BUG: memory leak
unreferenced object 0xffff888116db0100 (size 256):
comm "syz-executor.0", pid 8883, jiffies 4294944321 (age 373.070s)

hex dump (first 32 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................

00 00 00 00 0a 00 00 00 00 00 f3 16 81 88 ff ff ................
backtrace:
[<000000009ef3a83b>] kmalloc include/linux/slab.h:552 [inline]
[<000000009ef3a83b>] kzalloc include/linux/slab.h:682 [inline]
[<000000009ef3a83b>] rxrpc_alloc_local net/rxrpc/local_object.c:79 [inline]
[<000000009ef3a83b>] rxrpc_lookup_local+0x1c1/0x760 net/rxrpc/local_object.c:244
[<0000000050950ced>] rxrpc_bind+0x174/0x240 net/rxrpc/af_rxrpc.c:149
[<000000000ac020bd>] afs_open_socket+0xdb/0x200 fs/afs/rxrpc.c:64
[<00000000e684bacb>] afs_net_init+0x2b4/0x340 fs/afs/main.c:126
[<000000005fdca026>] ops_init+0x4e/0x190 net/core/net_namespace.c:152
[<000000009edc1bfd>] setup_net+0xde/0x2d0 net/core/net_namespace.c:342
[<0000000022d18a10>] copy_net_ns+0x19f/0x3e0 net/core/net_namespace.c:483
[<00000000775d29a1>] create_new_namespaces+0x199/0x4f0 kernel/nsproxy.c:110
[<00000000ea080f0c>] unshare_nsproxy_namespaces+0x9b/0x120 kernel/nsproxy.c:226
[<000000002c51c8c8>] ksys_unshare+0x2fe/0x5c0 kernel/fork.c:2957
[<00000000177a9d84>] __do_sys_unshare kernel/fork.c:3025 [inline]
[<00000000177a9d84>] __se_sys_unshare kernel/fork.c:3023 [inline]
[<00000000177a9d84>] __x64_sys_unshare+0x12/0x20 kernel/fork.c:3023
[<000000000e9763c5>] do_syscall_64+0x2d/0x70 arch/x86/entry/common.c:46
[<00000000fd35138e>] entry_SYSCALL_64_after_hwframe+0x44/0xa9



Tested on:

commit: b61f738a can: mcba_usb: Fix memory leak when cancelling urb
git tree: https://github.com/minhbq-99/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=140bf0af500000
kernel config: https://syzkaller.appspot.com/x/.config?x=ed1c642581af5b27

[Bui Quang Minh]

In mcba_usb_read_bulk_callback(), when we don't resubmit or fails to

resubmit the urb, we need to deallocate the transfer buffer that is
allocated in mcba_usb_start().

Reported-by: syzbot+57281c...@syzkaller.appspotmail.com
Signed-off-by: Bui Quang Minh <minhqua...@gmail.com>
---

v1: add memory leak fix when not resubmitting urb
v2: add memory leak fix when failing to resubmit urb

drivers/net/can/usb/mcba_usb.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index df54eb7d4b36..30236e640116 100644

--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -584,6 +584,8 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
case -EPIPE:
case -EPROTO:
case -ESHUTDOWN:
+ usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+ urb->transfer_buffer, urb->transfer_dma);
return;

default:

@@ -615,11 +617,14 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)

retval = usb_submit_urb(urb, GFP_ATOMIC);

- if (retval == -ENODEV)
- netif_device_detach(netdev);
- else if (retval)
+ if (retval < 0) {
netdev_err(netdev, "failed resubmitting read bulk urb: %d\n",
retval);

+ usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+ urb->transfer_buffer, urb->transfer_dma);

+ if (retval == -ENODEV)
+ netif_device_detach(netdev);
+ }
}

/* Start USB device */
--
2.17.1

[Oliver Neukum]

Am Montag, den 11.01.2021, 10:49 +0000 schrieb Bui Quang Minh:
> In mcba_usb_read_bulk_callback(), when we don't resubmit or fails to
> resubmit the urb, we need to deallocate the transfer buffer that is
> allocated in mcba_usb_start().
>
> Reported-by: syzbot+57281c...@syzkaller.appspotmail.com
> Signed-off-by: Bui Quang Minh <minhqua...@gmail.com>
> ---
> v1: add memory leak fix when not resubmitting urb
> v2: add memory leak fix when failing to resubmit urb
>
> drivers/net/can/usb/mcba_usb.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
> index df54eb7d4b36..30236e640116 100644
> --- a/drivers/net/can/usb/mcba_usb.c
> +++ b/drivers/net/can/usb/mcba_usb.c
> @@ -584,6 +584,8 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)
> case -EPIPE:
> case -EPROTO:
> case -ESHUTDOWN:
> + usb_free_coherent(urb->dev, urb->transfer_buffer_length,
> + urb->transfer_buffer, urb->transfer_dma);
> return;
>

Can you call usb_free_coherent() in what can be hard IRQ context?

Regards
Oliver

[Bui Quang Minh]

You are right, I digged in the code and saw some comments that on some
architectures, usb_free_coherent() cannot be called in hard IRQ context.
I see the usb_free_coherent() is called in write_bulk_callback too. I will
send a patch that uses usb_anchor to keep track of these urbs and cleanup
the transfer buffer later in disconnect().

Thank you for your review,
Quang Minh.

[Bui Quang Minh]

In mcba_usb_read_bulk_callback(), when we don't resubmit or fails to
resubmit the urb, we need to deallocate the transfer buffer that is
allocated in mcba_usb_start().

On some architectures, usb_free_coherent() cannot be called in interrupt
context, create a usb_anchor to keep track of these urbs and free them
later.

Reported-by: syzbot+57281c...@syzkaller.appspotmail.com
Signed-off-by: Bui Quang Minh <minhqua...@gmail.com>
---
v1: add memory leak fix when not resubmitting urb
v2: add memory leak fix when failing to resubmit urb

v3: remove usb_free_coherent() calls in interrupt context

drivers/net/can/usb/mcba_usb.c | 48 +++++++++++++++++++++++++++-------
1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/drivers/net/can/usb/mcba_usb.c b/drivers/net/can/usb/mcba_usb.c
index df54eb7d4b36..8025db484a22 100644
--- a/drivers/net/can/usb/mcba_usb.c
+++ b/drivers/net/can/usb/mcba_usb.c
@@ -77,6 +77,7 @@ struct mcba_priv {
struct net_device *netdev;
struct usb_anchor tx_submitted;
struct usb_anchor rx_submitted;
+ struct usb_anchor urbs_cleanup;
struct can_berr_counter bec;
bool usb_ka_first_pass;
bool can_ka_first_pass;
@@ -220,14 +221,17 @@ static void mcba_usb_write_bulk_callback(struct urb *urb)
{
struct mcba_usb_ctx *ctx = urb->context;
struct net_device *netdev;
+ struct mcba_priv *priv;

WARN_ON(!ctx);

netdev = ctx->priv->netdev;
+ priv = netdev_priv(netdev);

- /* free up our allocated buffer */
- usb_free_coherent(urb->dev, urb->transfer_buffer_length,
- urb->transfer_buffer, urb->transfer_dma);
+ /* On some architectures, usb_free_coherent() cannot be called in
+ * interrupt context, queue the urb for later cleanup
+ */
+ usb_anchor_urb(urb, &priv->urbs_cleanup);

if (ctx->can) {
if (!netif_device_present(netdev))
@@ -291,8 +295,11 @@ static netdev_tx_t mcba_usb_xmit(struct mcba_priv *priv,

failed:
usb_unanchor_urb(urb);
- usb_free_coherent(priv->udev, MCBA_USB_TX_BUFF_SIZE, buf,
- urb->transfer_dma);
+
+ /* On some architectures, usb_free_coherent() cannot be called in
+ * interrupt context, queue the urb for later cleanup
+ */
+ usb_anchor_urb(urb, &priv->urbs_cleanup);

if (err == -ENODEV)
netif_device_detach(priv->netdev);
@@ -584,7 +591,7 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)

case -EPIPE:
case -EPROTO:
case -ESHUTDOWN:

- return;
+ goto free;

default:
netdev_info(netdev, "Rx URB aborted (%d)\n", urb->status);
@@ -615,11 +622,20 @@ static void mcba_usb_read_bulk_callback(struct urb *urb)

retval = usb_submit_urb(urb, GFP_ATOMIC);

- if (retval == -ENODEV)
- netif_device_detach(netdev);
- else if (retval)
+ if (retval < 0) {
netdev_err(netdev, "failed resubmitting read bulk urb: %d\n",
retval);

+ if (retval == -ENODEV)
+ netif_device_detach(netdev);

+ goto free;
+ }
+
+ return;
+free:
+ /* On some architectures, usb_free_coherent() cannot be called in
+ * interrupt context, queue the urb for later cleanup
+ */
+ usb_anchor_urb(urb, &priv->urbs_cleanup);

}

/* Start USB device */

@@ -706,6 +722,17 @@ static int mcba_usb_open(struct net_device *netdev)
return 0;
}

+static void mcba_urb_cleanup(struct mcba_priv *priv)
+{
+ struct urb *urb;
+
+ while ((urb = usb_get_from_anchor(&priv->urbs_cleanup))) {

+ usb_free_coherent(urb->dev, urb->transfer_buffer_length,
+ urb->transfer_buffer, urb->transfer_dma);

+ usb_free_urb(urb);
+ }
+}
+
static void mcba_urb_unlink(struct mcba_priv *priv)
{
usb_kill_anchored_urbs(&priv->rx_submitted);
@@ -723,6 +750,7 @@ static int mcba_usb_close(struct net_device *netdev)

/* Stop polling */
mcba_urb_unlink(priv);
+ mcba_urb_cleanup(priv);

close_candev(netdev);
can_led_event(netdev, CAN_LED_EVENT_STOP);
@@ -812,6 +840,7 @@ static int mcba_usb_probe(struct usb_interface *intf,

init_usb_anchor(&priv->rx_submitted);
init_usb_anchor(&priv->tx_submitted);
+ init_usb_anchor(&priv->urbs_cleanup);

usb_set_intfdata(intf, priv);

@@ -877,6 +906,7 @@ static void mcba_usb_disconnect(struct usb_interface *intf)

unregister_candev(priv->netdev);
mcba_urb_unlink(priv);
+ mcba_urb_cleanup(priv);
free_candev(priv->netdev);
}

--
2.17.1

[Minh Bùi Quang]

Hi, I have sent a version 3 patch. However, I found out that usb_free_coherent()
is ok in this situation. In usb_free_coherent(), if the buffer is allocated via
dma_alloc_coherent() in usb_alloc_coherent(), dma_free_coherent() is called.
In dma_free_coherent(), ops->free() may be called in some cases which may
contains calls to vunmap() that is not permitted in interrupt context. However,
in usb_alloc_coherent(), buffer can be allocated from dma pool if the
size is less
than 2048 and the buffer size in mcba_usb is obviously less than 2048.
As a result,
usb_free_coherent() will at most fall in the path that calls
dma_pool_free(), which is
safe. Am I right?

Thanks,
Quang Minh.

[Bui Quang Minh]

Hi, I'm CC'ing CAN network driver maintainers so we can discuss the patch properly.

I'm so sorry for spamming emails.

Thanks,
Quang Minh.

[Павел Скрипкин]

Hi! 

Is this bug still not fixed? I wrote a patch, which is tested by syzbot

https://linux.googlesource.com/linux/kernel/git/torvalds/linux/+/82af93b5ae465171963d879f5c9eb27066c75012%5E!

BTW, sorry for this gmail formatting, I didn't find a way how to reply to google groups

via email client :)

With regards,
Pavel Skripkin

четверг, 21 января 2021 г. в 18:20:09 UTC+3, Bui Quang Minh: