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

Subsystems: the rest, virtio core

STALE2031d

3 messages, 2 authors, 2021-02-15 · open the first message on its own page

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

From: Parav Pandit <hidden>
Date: 2021-02-13 18:39:40 

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 <redacted>
---
 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

From: Stefano Garzarella <sgarzare@redhat.com>
Date: 2021-02-15 11:46:02 

On Sat, Feb 13, 2021 at 08:39:19PM +0200, Parav Pandit wrote:
quoted hunk
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 <redacted>
---
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

From: Parav Pandit <hidden>
Date: 2021-02-15 14:12:32 

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:
quoted
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 <redacted>
---
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)
quoted
	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);
quoted
	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




    
  

  
    

      

        Keyboard shortcuts
      

      

        
          
hback out one level

          
jnext message in thread

          
kprevious message in thread

          
ldrill in

          
Escclose help / fold thread tree

          
?toggle this help