[Qemu-devel] [PATCH] piix: fix resource leak reported by Coverity

[Qemu-devel] [PATCH] piix: fix resource leak reported by Coverity

[zhanghailiang]

config_fd should be closed before return, or there will
be a resource leak error.

Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
---
 hw/pci-host/piix.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 1fb71c8..7b2fbf9 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -764,6 +764,7 @@ static int host_pci_config_read(int pos, int len, uint32_t val)
     /* Access real host bridge. */
     int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
                       0, 0, 0, 0, "config");
+    int ret = 0;
 
     if (rc >= size || rc < 0) {
         return -ENODEV;
@@ -775,16 +776,18 @@ static int host_pci_config_read(int pos, int len, uint32_t val)
     }
 
     if (lseek(config_fd, pos, SEEK_SET) != pos) {
-        return -errno;
+        ret = -errno;
+        goto out;
     }
     do {
         rc = read(config_fd, (uint8_t *)&val, len);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
     if (rc != len) {
-        return -errno;
+        ret = -errno;
     }
-
-    return 0;
+out:
+    close(config_fd);
+    return ret;
 }
 
 static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH] piix: fix resource leak reported by Coverity

[Stefan Hajnoczi]

On Mon, Sep 14, 2015 at 06:40:10PM +0800, zhanghailiang wrote:
> config_fd should be closed before return, or there will
> be a resource leak error.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  hw/pci-host/piix.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Re: [Qemu-devel] [PATCH] piix: fix resource leak reported by Coverity

[Michael Tokarev]

[-- Attachment #1: Type: text/plain, Size: 1418 bytes --]

14.09.2015 13:40, zhanghailiang wrote:
> config_fd should be closed before return, or there will
> be a resource leak error.
> 
> Signed-off-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
> ---
>  hw/pci-host/piix.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index 1fb71c8..7b2fbf9 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -764,6 +764,7 @@ static int host_pci_config_read(int pos, int len, uint32_t val)
>      /* Access real host bridge. */
>      int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
>                        0, 0, 0, 0, "config");
> +    int ret = 0;
>  
>      if (rc >= size || rc < 0) {
>          return -ENODEV;
> @@ -775,16 +776,18 @@ static int host_pci_config_read(int pos, int len, uint32_t val)
>      }
>  
>      if (lseek(config_fd, pos, SEEK_SET) != pos) {
> -        return -errno;
> +        ret = -errno;
> +        goto out;
>      }
>      do {
>          rc = read(config_fd, (uint8_t *)&val, len);
>      } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>      if (rc != len) {
> -        return -errno;
> +        ret = -errno;
>      }
> -
> -    return 0;
> +out:
> +    close(config_fd);
> +    return ret;
>  }
>  
>  static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)

How about the attached (untested)?

Thanks,

/mjt


[-- Attachment #2: piix-fix-resource-leak.diff --]
[-- Type: text/x-patch, Size: 1114 bytes --]

diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index 7b2fbf9..42f847d 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -764,7 +764,6 @@ static int host_pci_config_read(int pos, int len, uint32_t val)
     /* Access real host bridge. */
     int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
                       0, 0, 0, 0, "config");
-    int ret = 0;
 
     if (rc >= size || rc < 0) {
         return -ENODEV;
@@ -774,20 +773,15 @@ static int host_pci_config_read(int pos, int len, uint32_t val)
     if (config_fd < 0) {
         return -ENODEV;
     }
-
-    if (lseek(config_fd, pos, SEEK_SET) != pos) {
-        ret = -errno;
-        goto out;
-    }
     do {
-        rc = read(config_fd, (uint8_t *)&val, len);
+        rc = pread(config_fd, (uint8_t *)&val, len, pos);
     } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
+    close(config_fd);
     if (rc != len) {
-        ret = -errno;
+        return -errno;
     }
-out:
-    close(config_fd);
-    return ret;
+
+    return 0;
 }
 
 static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)