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] [day] [month] [year] [list]

Date:   Fri, 4 Nov 2022 10:05:02 -0700
From:   Dmitry Vyukov <dvyukov@...gle.com>
To:     bongsu.jeon@...sung.com
Cc:     "leon@...nel.org" <leon@...nel.org>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "syzkaller@...glegroups.com" <syzkaller@...glegroups.com>
Subject: Re: [PATCH net-next v2] nfc: Allow to create multiple virtual nci devices

On Thu, 3 Nov 2022 at 17:59, Bongsu Jeon <bongsu.jeon@...sung.com> wrote:
> 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?

You are right. Sent v3 with the fix.