[PATCH] vdpa: Fix memory leak in error unwinding path

[PATCH] vdpa: Fix memory leak in error unwinding path

[Parav Pandit]

When device get command fails to find the device or mdev,
it skips to free the skb during error unwinding path.
Fix it by freeing in error unwind path.

Fixes: a12a2f694ce8 ("vdpa: Enable user to query vdpa device info")
Signed-off-by: Parav Pandit <parav@nvidia.com>
---
 drivers/vdpa/vdpa.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 3d997b389345..e3f1bfdf8d6f 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -540,20 +540,22 @@ static int vdpa_nl_cmd_dev_get_doit(struct sk_buff *skb, struct genl_info *info)
 	if (!dev) {
 		mutex_unlock(&vdpa_dev_mutex);
 		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
-		return -ENODEV;
+		err = -ENODEV;
+		goto err;
 	}
 	vdev = container_of(dev, struct vdpa_device, dev);
 	if (!vdev->mdev) {
 		mutex_unlock(&vdpa_dev_mutex);
 		put_device(dev);
-		return -EINVAL;
+		err = -EINVAL;
+		goto err;
 	}
 	err = vdpa_dev_fill(vdev, msg, info->snd_portid, info->snd_seq, 0, info->extack);
 	if (!err)
 		err = genlmsg_reply(msg, info);
 	put_device(dev);
 	mutex_unlock(&vdpa_dev_mutex);
-
+err:
 	if (err)
 		nlmsg_free(msg);
 	return err;
-- 
2.26.2

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] vdpa: Fix memory leak in error unwinding path

[Stefano Garzarella]

On Sat, Feb 13, 2021 at 08:39:19PM +0200, Parav Pandit wrote:
>When device get command fails to find the device or mdev,
>it skips to free the skb during error unwinding path.
>Fix it by freeing in error unwind path.
>
>Fixes: a12a2f694ce8 ("vdpa: Enable user to query vdpa device info")
>Signed-off-by: Parav Pandit <parav@nvidia.com>
>---
> drivers/vdpa/vdpa.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
>index 3d997b389345..e3f1bfdf8d6f 100644
>--- a/drivers/vdpa/vdpa.c
>+++ b/drivers/vdpa/vdpa.c
>@@ -540,20 +540,22 @@ static int vdpa_nl_cmd_dev_get_doit(struct sk_buff *skb, struct genl_info *info)
> 	if (!dev) {
> 		mutex_unlock(&vdpa_dev_mutex);
> 		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
>-		return -ENODEV;
>+		err = -ENODEV;
>+		goto err;
> 	}
> 	vdev = container_of(dev, struct vdpa_device, dev);
> 	if (!vdev->mdev) {
> 		mutex_unlock(&vdpa_dev_mutex);
> 		put_device(dev);
>-		return -EINVAL;
>+		err = -EINVAL;
>+		goto err;
> 	}
> 	err = vdpa_dev_fill(vdev, msg, info->snd_portid, info->snd_seq, 0, info->extack);
> 	if (!err)
> 		err = genlmsg_reply(msg, info);
> 	put_device(dev);
> 	mutex_unlock(&vdpa_dev_mutex);
>-
>+err:

If we put this label before mutex_unlock(), we can remove that call in 
the error paths.

Maybe we can also add another label before put_device() and jump to it 
in the "if (!vdev->mdev)" case.

Thanks,
Stefano

> 	if (err)
> 		nlmsg_free(msg);
> 	return err;
>-- 
>2.26.2
>
>_______________________________________________
>Virtualization mailing list
>Virtualization@lists.linux-foundation.org
>https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

RE: [PATCH] vdpa: Fix memory leak in error unwinding path

[Parav Pandit]

> From: Stefano Garzarella <sgarzare@redhat.com>
> Sent: Monday, February 15, 2021 5:16 PM
> 
> On Sat, Feb 13, 2021 at 08:39:19PM +0200, Parav Pandit wrote:
> >When device get command fails to find the device or mdev, it skips to
> >free the skb during error unwinding path.
> >Fix it by freeing in error unwind path.
> >
> >Fixes: a12a2f694ce8 ("vdpa: Enable user to query vdpa device info")
> >Signed-off-by: Parav Pandit <parav@nvidia.com>
> >---
> > drivers/vdpa/vdpa.c | 8 +++++---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> >3d997b389345..e3f1bfdf8d6f 100644
> >--- a/drivers/vdpa/vdpa.c
> >+++ b/drivers/vdpa/vdpa.c
> >@@ -540,20 +540,22 @@ static int vdpa_nl_cmd_dev_get_doit(struct
> sk_buff *skb, struct genl_info *info)
> > 	if (!dev) {
> > 		mutex_unlock(&vdpa_dev_mutex);
> > 		NL_SET_ERR_MSG_MOD(info->extack, "device not found");
> >-		return -ENODEV;
> >+		err = -ENODEV;
> >+		goto err;
> > 	}
> > 	vdev = container_of(dev, struct vdpa_device, dev);
> > 	if (!vdev->mdev) {
> > 		mutex_unlock(&vdpa_dev_mutex);
> > 		put_device(dev);
> >-		return -EINVAL;
> >+		err = -EINVAL;
> >+		goto err;
> > 	}
> > 	err = vdpa_dev_fill(vdev, msg, info->snd_portid, info->snd_seq, 0,
> info->extack);
> > 	if (!err)
> > 		err = genlmsg_reply(msg, info);
> > 	put_device(dev);
> > 	mutex_unlock(&vdpa_dev_mutex);
> >-
> >+err:
> 
> If we put this label before mutex_unlock(), we can remove that call in the
> error paths.
> 
> Maybe we can also add another label before put_device() and jump to it in
> the "if (!vdev->mdev)" case.
> 
Yep, I will send v2.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization