All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
@ 2022-10-17  8:44 Shang XiaoJing
  2022-10-18  9:32 ` Paolo Abeni
  0 siblings, 1 reply; 4+ messages in thread
From: Shang XiaoJing @ 2022-10-17  8:44 UTC (permalink / raw)
  To: bongsu.jeon, krzysztof.kozlowski, kuba, 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>
---
 drivers/nfc/virtual_ncidev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
index f577449e4935..9f54a4e0eb51 100644
--- a/drivers/nfc/virtual_ncidev.c
+++ b/drivers/nfc/virtual_ncidev.c
@@ -64,6 +64,7 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
 	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] 4+ messages in thread
* Re: [PATCH] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
  2022-10-17  8:44 [PATCH] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send() Shang XiaoJing
@ 2022-10-18  9:32 ` Paolo Abeni
  2022-10-18  9:38   ` shangxiaojing
  2022-10-18 13:55   ` Krzysztof Kozlowski
  0 siblings, 2 replies; 4+ messages in thread
From: Paolo Abeni @ 2022-10-18  9:32 UTC (permalink / raw)
  To: Shang XiaoJing, bongsu.jeon, krzysztof.kozlowski, kuba, netdev

Hello,

On Mon, 2022-10-17 at 16:44 +0800, 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>
> ---
>  drivers/nfc/virtual_ncidev.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
> index f577449e4935..9f54a4e0eb51 100644
> --- a/drivers/nfc/virtual_ncidev.c
> +++ b/drivers/nfc/virtual_ncidev.c
> @@ -64,6 +64,7 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
>  	send_buff = skb_copy(skb, GFP_KERNEL);
>  	mutex_unlock(&nci_mutex);
>  	wake_up_interruptible(&wq);
> +	consume_skb(skb);

Looking at the nci core, it seems to me that the send op takes full
ownership of the skb argument. That is, you need to additionally free
it even on error paths.

@Krzysztof: it looks like that the send() return value is always
ignored. Do you plan to use it at some point or could we change the
return type to void?

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
  2022-10-18  9:32 ` Paolo Abeni
@ 2022-10-18  9:38   ` shangxiaojing
  2022-10-18 13:55   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 4+ messages in thread
From: shangxiaojing @ 2022-10-18  9:38 UTC (permalink / raw)
  To: Paolo Abeni, bongsu.jeon, krzysztof.kozlowski, kuba, netdev



On 2022/10/18 17:32, Paolo Abeni wrote:
> Hello,
> 
> On Mon, 2022-10-17 at 16:44 +0800, 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>
>> ---
>>   drivers/nfc/virtual_ncidev.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
>> index f577449e4935..9f54a4e0eb51 100644
>> --- a/drivers/nfc/virtual_ncidev.c
>> +++ b/drivers/nfc/virtual_ncidev.c
>> @@ -64,6 +64,7 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
>>   	send_buff = skb_copy(skb, GFP_KERNEL);
>>   	mutex_unlock(&nci_mutex);
>>   	wake_up_interruptible(&wq);
>> +	consume_skb(skb);
> 
> Looking at the nci core, it seems to me that the send op takes full
> ownership of the skb argument. That is, you need to additionally free
> it even on error paths.
right, will be fixed in patch v2.

Thanks,
Shang XiaoJing

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send()
  2022-10-18  9:32 ` Paolo Abeni
  2022-10-18  9:38   ` shangxiaojing
@ 2022-10-18 13:55   ` Krzysztof Kozlowski
  1 sibling, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2022-10-18 13:55 UTC (permalink / raw)
  To: Paolo Abeni, Shang XiaoJing, bongsu.jeon, kuba, netdev

On 18/10/2022 05:32, Paolo Abeni wrote:
> @Krzysztof: it looks like that the send() return value is always
> ignored. Do you plan to use it at some point or could we change the
> return type to void?

Are you sure it is ignored? I think it is being returned in
nci_send_frame().

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-18 13:55 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-17  8:44 [PATCH] nfc: virtual_ncidev: Fix memory leak in virtual_nci_send() Shang XiaoJing
2022-10-18  9:32 ` Paolo Abeni
2022-10-18  9:38   ` shangxiaojing
2022-10-18 13:55   ` Krzysztof Kozlowski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.