netdev - RE: [PATCH net-next v2] nfc: Allow to create multiple virtual nci devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20221104005917epcms2p228981fa87d326a8d4f503911f3472703@epcms2p2>
Date:   Fri, 04 Nov 2022 09:59:17 +0900
From:   Bongsu Jeon <bongsu.jeon@...sung.com>
To:     Dmitry Vyukov <dvyukov@...gle.com>,
        "leon@...nel.org" <leon@...nel.org>,
        Bongsu Jeon <bongsu.jeon@...sung.com>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
CC:     "syzkaller@...glegroups.com" <syzkaller@...glegroups.com>
Subject: RE: [PATCH net-next v2] nfc: Allow to create multiple virtual nci
 devices


On Fri, Nov 4, 2022 at 3:19 AM Dmitry Vyukov<dvyukov@...gle.com> wrote:
>
>The current virtual nci driver is great for testing and fuzzing.
>But it allows to create at most one "global" device which does not allow
>to run parallel tests and harms fuzzing isolation and reproducibility.
>Restructure the driver to allow creation of multiple independent devices.
>This should be backwards compatible for existing tests.
>
>Signed-off-by: Dmitry Vyukov <dvyukov@...gle.com>
>Cc: Bongsu Jeon <bongsu.jeon@...sung.com>
>Cc: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
>Cc: netdev@...r.kernel.org
>
>---
>Changes in v2:
> - check return value of skb_clone()
> - rebase onto currnet net-next
>---
> drivers/nfc/virtual_ncidev.c | 146 +++++++++++++++++------------------
> 1 file changed, 70 insertions(+), 76 deletions(-)
>
>diff --git a/drivers/nfc/virtual_ncidev.c b/drivers/nfc/virtual_ncidev.c
>index 85c06dbb2c449..48d6d09e2f6fd 100644
>--- a/drivers/nfc/virtual_ncidev.c
>+++ b/drivers/nfc/virtual_ncidev.c
>@@ -13,12 +13,6 @@

<...>

> static int virtual_ncidev_open(struct inode *inode, struct file *file)
> {
> 	int ret = 0;
>+	struct virtual_nci_dev *vdev;
> 
>-	mutex_lock(&nci_mutex);
>-	if (state != virtual_ncidev_disabled) {
>-		mutex_unlock(&nci_mutex);
>-		return -EBUSY;
>-	}
>-
>-	ndev = nci_allocate_device(&virtual_nci_ops, VIRTUAL_NFC_PROTOCOLS,
>-				   0, 0);
>-	if (!ndev) {
>-		mutex_unlock(&nci_mutex);
>+	vdev = kzalloc(sizeof(*vdev), GFP_KERNEL);
>+	if (!vdev)
>+		return -ENOMEM;
>+	vdev->ndev = nci_allocate_device(&virtual_nci_ops,
>+		VIRTUAL_NFC_PROTOCOLS, 0, 0);
>+	if (!vdev->ndev) {
>+		kfree(vdev);
> 		return -ENOMEM;
> 	}
> 
>-	ret = nci_register_device(ndev);
>+	mutex_init(&vdev->mtx);
>+	init_waitqueue_head(&vdev->wq);
>+	file->private_data = vdev;
>+	nci_set_drvdata(vdev->ndev, vdev);
>+
>+	ret = nci_register_device(vdev->ndev);
> 	if (ret < 0) {
>-		nci_free_device(ndev);
>-		mutex_unlock(&nci_mutex);
>+		mutex_destroy(&vdev->mtx);
>+		nci_free_device(vdev->ndev);
>+		kfree(vdev);
> 		return ret;
> 	}
>-	state = virtual_ncidev_enabled;
>-	mutex_unlock(&nci_mutex);
> 
> 	return 0;
> }
> 
> static int virtual_ncidev_close(struct inode *inode, struct file *file)
> {
>-	mutex_lock(&nci_mutex);
>-
>-	if (state == virtual_ncidev_enabled) {
>-		state = virtual_ncidev_disabling;
>-		mutex_unlock(&nci_mutex);
>+	struct virtual_nci_dev *vdev = file->private_data;
> 
>-		nci_unregister_device(ndev);
>-		nci_free_device(ndev);
>-
>-		mutex_lock(&nci_mutex);
>-	}
>-
>-	state = virtual_ncidev_disabled;
>-	mutex_unlock(&nci_mutex);
>+	nci_unregister_device(vdev->ndev);
>+	nci_free_device(vdev->ndev);
>+	mutex_destroy(&vdev->mtx);

    Isn't kfree(vdev) necessary?

> 
> 	return 0;
> }
>