[PATCH] vfio/iommufd: Fix memory leak

qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed

* [PATCH] vfio/iommufd: Fix memory leak
@ 2024-03-13 21:06 Cédric Le Goater
  2024-03-13 21:11 ` Eric Auger
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Cédric Le Goater @ 2024-03-13 21:06 UTC (permalink / raw)
  To: qemu-devel
  Cc: Alex Williamson, Cédric Le Goater, Eric Auger, Yi Liu,
	Zhenzhong Duan

Make sure variable contents is freed if scanf fails.

Cc: Eric Auger <eric.auger@redhat.com>
Cc: Yi Liu <yi.l.liu@intel.com>
Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
Fixes: CID 1540007
Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
Signed-off-by: Cédric Le Goater <clg@redhat.com>
---
 hw/vfio/iommufd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c51cc96153762a6bc8550 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
 
     if (sscanf(contents, "%d:%d", &major, &minor) != 2) {
         error_setg(errp, "failed to get major:minor for \"%s\"", vfio_dev_path);
-        goto out_free_dev_path;
+        goto out_free_contents;
     }
-    g_free(contents);
     vfio_devt = makedev(major, minor);
 
     vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
@@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
     trace_iommufd_cdev_getfd(vfio_path, ret);
     g_free(vfio_path);
 
+out_free_contents:
+    g_free(contents);
 out_free_dev_path:
     g_free(vfio_dev_path);
 out_close_dir:
-- 
2.44.0



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfio/iommufd: Fix memory leak
  2024-03-13 21:06 [PATCH] vfio/iommufd: Fix memory leak Cédric Le Goater
@ 2024-03-13 21:11 ` Eric Auger
  2024-03-14  3:25 ` Yi Liu
  2024-03-14  3:31 ` Duan, Zhenzhong
  2 siblings, 0 replies; 5+ messages in thread
From: Eric Auger @ 2024-03-13 21:11 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel; +Cc: Alex Williamson, Yi Liu, Zhenzhong Duan



On 3/13/24 22:06, Cédric Le Goater wrote:
> Make sure variable contents is freed if scanf fails.
>
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Yi Liu <yi.l.liu@intel.com>
> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Fixes: CID 1540007
> Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks!

Eric
> ---
>  hw/vfio/iommufd.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c51cc96153762a6bc8550 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
>  
>      if (sscanf(contents, "%d:%d", &major, &minor) != 2) {
>          error_setg(errp, "failed to get major:minor for \"%s\"", vfio_dev_path);
> -        goto out_free_dev_path;
> +        goto out_free_contents;
>      }
> -    g_free(contents);
>      vfio_devt = makedev(major, minor);
>  
>      vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
> @@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
>      trace_iommufd_cdev_getfd(vfio_path, ret);
>      g_free(vfio_path);
>  
> +out_free_contents:
> +    g_free(contents);
>  out_free_dev_path:
>      g_free(vfio_dev_path);
>  out_close_dir:



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfio/iommufd: Fix memory leak
  2024-03-13 21:06 [PATCH] vfio/iommufd: Fix memory leak Cédric Le Goater
  2024-03-13 21:11 ` Eric Auger
@ 2024-03-14  3:25 ` Yi Liu
  2024-03-14  3:31 ` Duan, Zhenzhong
  2 siblings, 0 replies; 5+ messages in thread
From: Yi Liu @ 2024-03-14  3:25 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel
  Cc: Alex Williamson, Eric Auger, Zhenzhong Duan

On 2024/3/14 05:06, Cédric Le Goater wrote:
> Make sure variable contents is freed if scanf fails.
> 
> Cc: Eric Auger <eric.auger@redhat.com>
> Cc: Yi Liu <yi.l.liu@intel.com>
> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Fixes: CID 1540007
> Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>   hw/vfio/iommufd.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c51cc96153762a6bc8550 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
>   
>       if (sscanf(contents, "%d:%d", &major, &minor) != 2) {
>           error_setg(errp, "failed to get major:minor for \"%s\"", vfio_dev_path);
> -        goto out_free_dev_path;
> +        goto out_free_contents;
>       }
> -    g_free(contents);
>       vfio_devt = makedev(major, minor);
>   
>       vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
> @@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp)
>       trace_iommufd_cdev_getfd(vfio_path, ret);
>       g_free(vfio_path);
>   
> +out_free_contents:
> +    g_free(contents);
>   out_free_dev_path:
>       g_free(vfio_dev_path);
>   out_close_dir:

good catch.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

-- 
Regards,
Yi Liu


^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH] vfio/iommufd: Fix memory leak
  2024-03-13 21:06 [PATCH] vfio/iommufd: Fix memory leak Cédric Le Goater
  2024-03-13 21:11 ` Eric Auger
  2024-03-14  3:25 ` Yi Liu
@ 2024-03-14  3:31 ` Duan, Zhenzhong
  2024-03-14  8:22   ` Cédric Le Goater
  2 siblings, 1 reply; 5+ messages in thread
From: Duan, Zhenzhong @ 2024-03-14  3:31 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-devel@nongnu.org
  Cc: Alex Williamson, Eric Auger, Liu, Yi L



>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Thursday, March 14, 2024 5:06 AM
>To: qemu-devel@nongnu.org
>Cc: Alex Williamson <alex.williamson@redhat.com>; Cédric Le Goater
><clg@redhat.com>; Eric Auger <eric.auger@redhat.com>; Liu, Yi L
><yi.l.liu@intel.com>; Duan, Zhenzhong <zhenzhong.duan@intel.com>
>Subject: [PATCH] vfio/iommufd: Fix memory leak
>
>Make sure variable contents is freed if scanf fails.
>
>Cc: Eric Auger <eric.auger@redhat.com>
>Cc: Yi Liu <yi.l.liu@intel.com>
>Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
>Fixes: CID 1540007
>Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
>Signed-off-by: Cédric Le Goater <clg@redhat.com>

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Unrelated to this patch, I see there are four g_free calls, not clear if it's deserved
to cleanup with g_autofree.

Thanks
Zhenzhong

>---
> hw/vfio/iommufd.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>index
>a75a785e90c64cdcc4d10c88d217801b3f536cdb..cd549e0ee8573e75772c5
>1cc96153762a6bc8550 100644
>--- a/hw/vfio/iommufd.c
>+++ b/hw/vfio/iommufd.c
>@@ -152,9 +152,8 @@ static int iommufd_cdev_getfd(const char
>*sysfs_path, Error **errp)
>
>     if (sscanf(contents, "%d:%d", &major, &minor) != 2) {
>         error_setg(errp, "failed to get major:minor for \"%s\"", vfio_dev_path);
>-        goto out_free_dev_path;
>+        goto out_free_contents;
>     }
>-    g_free(contents);
>     vfio_devt = makedev(major, minor);
>
>     vfio_path = g_strdup_printf("/dev/vfio/devices/%s", dent->d_name);
>@@ -166,6 +165,8 @@ static int iommufd_cdev_getfd(const char
>*sysfs_path, Error **errp)
>     trace_iommufd_cdev_getfd(vfio_path, ret);
>     g_free(vfio_path);
>
>+out_free_contents:
>+    g_free(contents);
> out_free_dev_path:
>     g_free(vfio_dev_path);
> out_close_dir:
>--
>2.44.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] vfio/iommufd: Fix memory leak
  2024-03-14  3:31 ` Duan, Zhenzhong
@ 2024-03-14  8:22   ` Cédric Le Goater
  0 siblings, 0 replies; 5+ messages in thread
From: Cédric Le Goater @ 2024-03-14  8:22 UTC (permalink / raw)
  To: Duan, Zhenzhong, qemu-devel@nongnu.org
  Cc: Alex Williamson, Eric Auger, Liu, Yi L

On 3/14/24 04:31, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Cédric Le Goater <clg@redhat.com>
>> Sent: Thursday, March 14, 2024 5:06 AM
>> To: qemu-devel@nongnu.org
>> Cc: Alex Williamson <alex.williamson@redhat.com>; Cédric Le Goater
>> <clg@redhat.com>; Eric Auger <eric.auger@redhat.com>; Liu, Yi L
>> <yi.l.liu@intel.com>; Duan, Zhenzhong <zhenzhong.duan@intel.com>
>> Subject: [PATCH] vfio/iommufd: Fix memory leak
>>
>> Make sure variable contents is freed if scanf fails.
>>
>> Cc: Eric Auger <eric.auger@redhat.com>
>> Cc: Yi Liu <yi.l.liu@intel.com>
>> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Fixes: CID 1540007
>> Fixes: 5ee3dc7af785 ("vfio/iommufd: Implement the iommufd backend")
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> 
> Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> Unrelated to this patch, I see there are four g_free calls, not clear if it's deserved
> to cleanup with g_autofree.

Ah yes. This is much better indeed.


Thanks,

C.





^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-03-14  8:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-13 21:06 [PATCH] vfio/iommufd: Fix memory leak Cédric Le Goater
2024-03-13 21:11 ` Eric Auger
2024-03-14  3:25 ` Yi Liu
2024-03-14  3:31 ` Duan, Zhenzhong
2024-03-14  8:22   ` Cédric Le Goater

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).