* [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
@ 2022-10-18 11:49 Shang XiaoJing
2022-10-18 14:06 ` Krzysztof Kozlowski
2022-10-20 0:33 ` Jakub Kicinski
0 siblings, 2 replies; 5+ messages in thread
From: Shang XiaoJing @ 2022-10-18 11:49 UTC (permalink / raw)
To: bongsu.jeon, krzysztof.kozlowski, kuba, pabeni, netdev; +Cc: shangxiaojing
skb should be free in virtual_nci_send(), otherwise kmemleak will report
memleak.
Steps for reproduction (simulated in qemu):
cd tools/testing/selftests/nci
make
./nci_dev
BUG: memory leak
unreferenced object 0xffff888107588000 (size 208):
comm "nci_dev", pid 206, jiffies 4294945376 (age 368.248s)
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:
[<000000008d94c8fd>] __alloc_skb+0x1da/0x290
[<00000000278bc7f8>] nci_send_cmd+0xa3/0x350
[<0000000081256a22>] nci_reset_req+0x6b/0xa0
[<000000009e721112>] __nci_request+0x90/0x250
[<000000005d556e59>] nci_dev_up+0x217/0x5b0
[<00000000e618ce62>] nfc_dev_up+0x114/0x220
[<00000000981e226b>] nfc_genl_dev_up+0x94/0xe0
[<000000009bb03517>] genl_family_rcv_msg_doit.isra.14+0x228/0x2d0
[<00000000b7f8c101>] genl_rcv_msg+0x35c/0x640
[<00000000c94075ff>] netlink_rcv_skb+0x11e/0x350
[<00000000440cfb1e>] genl_rcv+0x24/0x40
[<0000000062593b40>] netlink_unicast+0x43f/0x640
[<000000001d0b13cc>] netlink_sendmsg+0x73a/0xbf0
[<000000003272487f>] __sys_sendto+0x324/0x370
[<00000000ef9f1747>] __x64_sys_sendto+0xdd/0x1b0
[<000000001e437841>] do_syscall_64+0x3f/0x90
Fixes: e624e6c3e777 ("nfc: Add a virtual nci device driver")
Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
---
changes in v2:
- free skb in error paths too.
---
drivers/nfc/virtual_ncidev.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
index f577449e4935..3a4ad95b40a7 100644
--- a/drivers/nfc/virtual_ncidev.c
+++ b/drivers/nfc/virtual_ncidev.c
@@ -54,16 +54,19 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
mutex_lock(&nci_mutex);
if (state != virtual_ncidev_enabled) {
mutex_unlock(&nci_mutex);
+ consume_skb(skb);
return 0;
}
if (send_buff) {
mutex_unlock(&nci_mutex);
+ consume_skb(skb);
return -1;
}
send_buff = skb_copy(skb, GFP_KERNEL);
mutex_unlock(&nci_mutex);
wake_up_interruptible(&wq);
+ consume_skb(skb);
return 0;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
2022-10-18 11:49 [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send() Shang XiaoJing
@ 2022-10-18 14:06 ` Krzysztof Kozlowski
2022-10-19 1:29 ` shangxiaojing
2022-10-20 0:33 ` Jakub Kicinski
1 sibling, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-18 14:06 UTC (permalink / raw)
To: Shang XiaoJing, bongsu.jeon, kuba, pabeni, netdev
On 18/10/2022 07:49, Shang XiaoJing wrote:
> skb should be free in virtual_nci_send(), otherwise kmemleak will report
> memleak.
>
> Steps for reproduction (simulated in qemu):
> cd tools/testing/selftests/nci
> make
> ./nci_dev
>
> BUG: memory leak
> unreferenced object 0xffff888107588000 (size 208):
> comm "nci_dev", pid 206, jiffies 4294945376 (age 368.248s)
> 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:
> [<000000008d94c8fd>] __alloc_skb+0x1da/0x290
> [<00000000278bc7f8>] nci_send_cmd+0xa3/0x350
> [<0000000081256a22>] nci_reset_req+0x6b/0xa0
> [<000000009e721112>] __nci_request+0x90/0x250
> [<000000005d556e59>] nci_dev_up+0x217/0x5b0
> [<00000000e618ce62>] nfc_dev_up+0x114/0x220
> [<00000000981e226b>] nfc_genl_dev_up+0x94/0xe0
> [<000000009bb03517>] genl_family_rcv_msg_doit.isra.14+0x228/0x2d0
> [<00000000b7f8c101>] genl_rcv_msg+0x35c/0x640
> [<00000000c94075ff>] netlink_rcv_skb+0x11e/0x350
> [<00000000440cfb1e>] genl_rcv+0x24/0x40
> [<0000000062593b40>] netlink_unicast+0x43f/0x640
> [<000000001d0b13cc>] netlink_sendmsg+0x73a/0xbf0
> [<000000003272487f>] __sys_sendto+0x324/0x370
> [<00000000ef9f1747>] __x64_sys_sendto+0xdd/0x1b0
> [<000000001e437841>] do_syscall_64+0x3f/0x90
>
> Fixes: e624e6c3e777 ("nfc: Add a virtual nci device driver")
> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
> ---
> changes in v2:
> - free skb in error paths too.
> ---
> drivers/nfc/virtual_ncidev.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
> index f577449e4935..3a4ad95b40a7 100644
> --- a/drivers/nfc/virtual_ncidev.c
> +++ b/drivers/nfc/virtual_ncidev.c
> @@ -54,16 +54,19 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
> mutex_lock(&nci_mutex);
> if (state != virtual_ncidev_enabled) {
> mutex_unlock(&nci_mutex);
> + consume_skb(skb);
Ehhh... This looks ok, but now I wonder why none of other NCI send
driver do it. If the finding is correct, all drivers have same issue.
> return 0;
> }
>
> if (send_buff) {
> mutex_unlock(&nci_mutex);
> + consume_skb(skb);
> return -1;
> }
> send_buff = skb_copy(skb, GFP_KERNEL);
> mutex_unlock(&nci_mutex);
> wake_up_interruptible(&wq);
> + consume_skb(skb);
This also looks ok and as well all drivers seem to be affected.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
2022-10-18 14:06 ` Krzysztof Kozlowski
@ 2022-10-19 1:29 ` shangxiaojing
0 siblings, 0 replies; 5+ messages in thread
From: shangxiaojing @ 2022-10-19 1:29 UTC (permalink / raw)
To: Krzysztof Kozlowski, bongsu.jeon, kuba, pabeni, netdev
On 2022/10/18 22:06, Krzysztof Kozlowski wrote:
> On 18/10/2022 07:49, Shang XiaoJing wrote:
>> skb should be free in virtual_nci_send(), otherwise kmemleak will report
>> memleak.
>>
>> Steps for reproduction (simulated in qemu):
>> cd tools/testing/selftests/nci
>> make
>> ./nci_dev
>>
>> BUG: memory leak
>> unreferenced object 0xffff888107588000 (size 208):
>> comm "nci_dev", pid 206, jiffies 4294945376 (age 368.248s)
>> 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:
>> [<000000008d94c8fd>] __alloc_skb+0x1da/0x290
>> [<00000000278bc7f8>] nci_send_cmd+0xa3/0x350
>> [<0000000081256a22>] nci_reset_req+0x6b/0xa0
>> [<000000009e721112>] __nci_request+0x90/0x250
>> [<000000005d556e59>] nci_dev_up+0x217/0x5b0
>> [<00000000e618ce62>] nfc_dev_up+0x114/0x220
>> [<00000000981e226b>] nfc_genl_dev_up+0x94/0xe0
>> [<000000009bb03517>] genl_family_rcv_msg_doit.isra.14+0x228/0x2d0
>> [<00000000b7f8c101>] genl_rcv_msg+0x35c/0x640
>> [<00000000c94075ff>] netlink_rcv_skb+0x11e/0x350
>> [<00000000440cfb1e>] genl_rcv+0x24/0x40
>> [<0000000062593b40>] netlink_unicast+0x43f/0x640
>> [<000000001d0b13cc>] netlink_sendmsg+0x73a/0xbf0
>> [<000000003272487f>] __sys_sendto+0x324/0x370
>> [<00000000ef9f1747>] __x64_sys_sendto+0xdd/0x1b0
>> [<000000001e437841>] do_syscall_64+0x3f/0x90
>>
>> Fixes: e624e6c3e777 ("nfc: Add a virtual nci device driver")
>> Signed-off-by: Shang XiaoJing <shangxiaojing@huawei.com>
>> ---
>> changes in v2:
>> - free skb in error paths too.
>> ---
>> drivers/nfc/virtual_ncidev.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
>> index f577449e4935..3a4ad95b40a7 100644
>> --- a/drivers/nfc/virtual_ncidev.c
>> +++ b/drivers/nfc/virtual_ncidev.c
>> @@ -54,16 +54,19 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
>> mutex_lock(&nci_mutex);
>> if (state != virtual_ncidev_enabled) {
>> mutex_unlock(&nci_mutex);
>> + consume_skb(skb);
>
> Ehhh... This looks ok, but now I wonder why none of other NCI send
> driver do it. If the finding is correct, all drivers have same issue.
yes, i'll try to reproduce these issues, and make another patch set if
there are the issues.
Thanks,
--
Shang XiaoJing
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
2022-10-18 11:49 [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send() Shang XiaoJing
2022-10-18 14:06 ` Krzysztof Kozlowski
@ 2022-10-20 0:33 ` Jakub Kicinski
2022-10-20 0:59 ` shangxiaojing
1 sibling, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2022-10-20 0:33 UTC (permalink / raw)
To: Shang XiaoJing; +Cc: bongsu.jeon, krzysztof.kozlowski, pabeni, netdev
On Tue, 18 Oct 2022 19:49:35 +0800 Shang XiaoJing wrote:
> --- a/drivers/nfc/virtual_ncidev.c
> +++ b/drivers/nfc/virtual_ncidev.c
> @@ -54,16 +54,19 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
> mutex_lock(&nci_mutex);
> if (state != virtual_ncidev_enabled) {
> mutex_unlock(&nci_mutex);
> + consume_skb(skb);
> return 0;
> }
>
> if (send_buff) {
> mutex_unlock(&nci_mutex);
> + consume_skb(skb);
> return -1;
these two should be kfree_skb() as we're dropping a packet
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
2022-10-20 0:33 ` Jakub Kicinski
@ 2022-10-20 0:59 ` shangxiaojing
0 siblings, 0 replies; 5+ messages in thread
From: shangxiaojing @ 2022-10-20 0:59 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: bongsu.jeon, krzysztof.kozlowski, pabeni, netdev
On 2022/10/20 8:33, Jakub Kicinski wrote:
> On Tue, 18 Oct 2022 19:49:35 +0800 Shang XiaoJing wrote:
>> --- a/drivers/nfc/virtual_ncidev.c
>> +++ b/drivers/nfc/virtual_ncidev.c
>> @@ -54,16 +54,19 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
>> mutex_lock(&nci_mutex);
>> if (state != virtual_ncidev_enabled) {
>> mutex_unlock(&nci_mutex);
>> + consume_skb(skb);
>> return 0;
>> }
>>
>> if (send_buff) {
>> mutex_unlock(&nci_mutex);
>> + consume_skb(skb);
>> return -1;
>
> these two should be kfree_skb() as we're dropping a packet
ok, will be fixed in v3.
Thanks,
--
Shang XiaoJing
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-10-20 1:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-18 11:49 [PATCH v2] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send() Shang XiaoJing
2022-10-18 14:06 ` Krzysztof Kozlowski
2022-10-19 1:29 ` shangxiaojing
2022-10-20 0:33 ` Jakub Kicinski
2022-10-20 0:59 ` shangxiaojing
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).