2023-11-21 07:54:34

by Nguyen Dinh Phi

[permalink] [raw]
Subject: [PATCH v2] nfc: virtual_ncidev: Add variable to check if ndev is running

syzbot reported an memory leak that happens when an skb is add to
send_buff after virtual nci closed.
This patch adds a variable to track if the ndev is running before
handling new skb in send function.

Signed-off-by: Nguyen Dinh Phi <[email protected]>
Reported-by: [email protected]
Closes: https://lore.kernel.org/lkml/[email protected]
---
V2:
- Remove unused macro.
- Re-adding a line that was removed wrongly.
drivers/nfc/virtual_ncidev.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
index b027be0b0b6f..590b038e449e 100644
--- a/drivers/nfc/virtual_ncidev.c
+++ b/drivers/nfc/virtual_ncidev.c
@@ -26,10 +26,14 @@ struct virtual_nci_dev {
struct mutex mtx;
struct sk_buff *send_buff;
struct wait_queue_head wq;
+ bool running;
};

static int virtual_nci_open(struct nci_dev *ndev)
{
+ struct virtual_nci_dev *vdev = nci_get_drvdata(ndev);
+
+ vdev->running = true;
return 0;
}

@@ -40,6 +44,7 @@ static int virtual_nci_close(struct nci_dev *ndev)
mutex_lock(&vdev->mtx);
kfree_skb(vdev->send_buff);
vdev->send_buff = NULL;
+ vdev->running = false;
mutex_unlock(&vdev->mtx);

return 0;
@@ -50,7 +55,7 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
struct virtual_nci_dev *vdev = nci_get_drvdata(ndev);

mutex_lock(&vdev->mtx);
- if (vdev->send_buff) {
+ if (vdev->send_buff || !vdev->running) {
mutex_unlock(&vdev->mtx);
kfree_skb(skb);
return -1;
--
2.39.2


2023-11-21 08:17:38

by Bongsu Jeon

[permalink] [raw]
Subject: Re: [PATCH v2] nfc: virtual_ncidev: Add variable to check if ndev is running

On 21/11/2023 16:54, Nguyen Dinh Phi wrote:
> syzbot reported an memory leak that happens when an skb is add to
> send_buff after virtual nci closed.
> This patch adds a variable to track if the ndev is running before
> handling new skb in send function.
>
> Signed-off-by: Nguyen Dinh Phi <[email protected]>
> Reported-by: [email protected]
> Closes: https://lore.kernel.org/lkml/[email protected]
> ---
> V2:
> - Remove unused macro.
> - Re-adding a line that was removed wrongly.
> drivers/nfc/virtual_ncidev.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
> index b027be0b0b6f..590b038e449e 100644
> --- a/drivers/nfc/virtual_ncidev.c
> +++ b/drivers/nfc/virtual_ncidev.c
> @@ -26,10 +26,14 @@ struct virtual_nci_dev {
> struct mutex mtx;
> struct sk_buff *send_buff;
> struct wait_queue_head wq;
> + bool running;
> };
>
> static int virtual_nci_open(struct nci_dev *ndev)
> {
> + struct virtual_nci_dev *vdev = nci_get_drvdata(ndev);
> +
> + vdev->running = true;
> return 0;
> }
>
> @@ -40,6 +44,7 @@ static int virtual_nci_close(struct nci_dev *ndev)
> mutex_lock(&vdev->mtx);
> kfree_skb(vdev->send_buff);
> vdev->send_buff = NULL;
> + vdev->running = false;
> mutex_unlock(&vdev->mtx);
>
> return 0;
> @@ -50,7 +55,7 @@ static int virtual_nci_send(struct nci_dev *ndev, struct sk_buff *skb)
> struct virtual_nci_dev *vdev = nci_get_drvdata(ndev);
>
> mutex_lock(&vdev->mtx);
> - if (vdev->send_buff) {
> + if (vdev->send_buff || !vdev->running) {
> mutex_unlock(&vdev->mtx);
> kfree_skb(skb);
> return -1;
> --
> 2.39.2


Reviewed-by: Bongsu Jeon

Best regards,
Bongsu

2023-11-21 09:01:05

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2] nfc: virtual_ncidev: Add variable to check if ndev is running

On 21/11/2023 08:53, Nguyen Dinh Phi wrote:
> syzbot reported an memory leak that happens when an skb is add to
> send_buff after virtual nci closed.
> This patch adds a variable to track if the ndev is running before
> handling new skb in send function.
>
> Signed-off-by: Nguyen Dinh Phi <[email protected]>
> Reported-by: [email protected]
> Closes: https://lore.kernel.org/lkml/[email protected]
> ---
> V2:
> - Remove unused macro.


Reviewed-by: Krzysztof Kozlowski <[email protected]>

Best regards,
Krzysztof

2023-11-22 11:01:32

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH v2] nfc: virtual_ncidev: Add variable to check if ndev is running

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Tue, 21 Nov 2023 15:53:57 +0800 you wrote:
> syzbot reported an memory leak that happens when an skb is add to
> send_buff after virtual nci closed.
> This patch adds a variable to track if the ndev is running before
> handling new skb in send function.
>
> Signed-off-by: Nguyen Dinh Phi <[email protected]>
> Reported-by: [email protected]
> Closes: https://lore.kernel.org/lkml/[email protected]
>
> [...]

Here is the summary with links:
- [v2] nfc: virtual_ncidev: Add variable to check if ndev is running
https://git.kernel.org/netdev/net/c/84d2db91f14a

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html