diff mbox

libvhost-user: Fix resource leak

Message ID 1516290116-4366-1-git-send-email-xieyongji@baidu.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yongji Xie Jan. 18, 2018, 3:41 p.m. UTC 
Free the mmaped memory when we need to mmap new memory
space on vu_set_mem_table_exec() and vu_set_log_base_exec() to
avoid memory leak.

Also close the corresponding fd after mmap() on
vu_set_log_base_exec() to avoid fd leak.

Signed-off-by: Yongji Xie <xieyongji@baidu.com>
---
 contrib/libvhost-user/libvhost-user.c |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Marc-André Lureau Jan. 18, 2018, 3:53 p.m. UTC | #1 
Hi

On Thu, Jan 18, 2018 at 4:41 PM, Yongji Xie <elohimes@gmail.com> wrote:
> Free the mmaped memory when we need to mmap new memory
> space on vu_set_mem_table_exec() and vu_set_log_base_exec() to
> avoid memory leak.
>
> Also close the corresponding fd after mmap() on
> vu_set_log_base_exec() to avoid fd leak.
>
> Signed-off-by: Yongji Xie <xieyongji@baidu.com>
> ---
>  contrib/libvhost-user/libvhost-user.c |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
> index f409bd3..cadced7 100644
> --- a/contrib/libvhost-user/libvhost-user.c
> +++ b/contrib/libvhost-user/libvhost-user.c
> @@ -405,6 +405,15 @@ vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
>  {
>      int i;
>      VhostUserMemory *memory = &vmsg->payload.memory;
> +
> +    for (i = 0; i < dev->nregions; i++) {
> +        VuDevRegion *r = &dev->regions[i];
> +        void *m = (void *) (uintptr_t) r->mmap_addr;
> +
> +        if (m) {
> +            munmap(m, r->size + r->mmap_offset);
> +        }
> +    }
>      dev->nregions = memory->nregions;
>
>      DPRINT("Nregions: %d\n", memory->nregions);
> @@ -470,9 +479,14 @@ vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
>
>      rc = mmap(0, log_mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
>                log_mmap_offset);
> +    close(fd);

or call vmsg_close_fds() ?

>      if (rc == MAP_FAILED) {
>          perror("log mmap error");
>      }
> +
> +    if (dev->log_table) {
> +        munmap(dev->log_table, dev->log_size);
> +    }
>      dev->log_table = rc;
>      dev->log_size = log_mmap_size;
>
> --
> 1.7.9.5
>
>


Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Maxime Coquelin Feb. 8, 2018, 4:37 p.m. UTC | #2 
On 01/18/2018 04:41 PM, Yongji Xie wrote:
> Free the mmaped memory when we need to mmap new memory
> space on vu_set_mem_table_exec() and vu_set_log_base_exec() to
> avoid memory leak.
> 
> Also close the corresponding fd after mmap() on
> vu_set_log_base_exec() to avoid fd leak.
> 
> Signed-off-by: Yongji Xie <xieyongji@baidu.com>
> ---
>   contrib/libvhost-user/libvhost-user.c |   14 ++++++++++++++
>   1 file changed, 14 insertions(+)

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime
diff mbox

Patch

diff --git a/contrib/libvhost-user/libvhost-user.c b/contrib/libvhost-user/libvhost-user.c
index f409bd3..cadced7 100644
--- a/contrib/libvhost-user/libvhost-user.c
+++ b/contrib/libvhost-user/libvhost-user.c
@@ -405,6 +405,15 @@  vu_set_mem_table_exec(VuDev *dev, VhostUserMsg *vmsg)
 {
     int i;
     VhostUserMemory *memory = &vmsg->payload.memory;
+
+    for (i = 0; i < dev->nregions; i++) {
+        VuDevRegion *r = &dev->regions[i];
+        void *m = (void *) (uintptr_t) r->mmap_addr;
+
+        if (m) {
+            munmap(m, r->size + r->mmap_offset);
+        }
+    }
     dev->nregions = memory->nregions;
 
     DPRINT("Nregions: %d\n", memory->nregions);
@@ -470,9 +479,14 @@  vu_set_log_base_exec(VuDev *dev, VhostUserMsg *vmsg)
 
     rc = mmap(0, log_mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd,
               log_mmap_offset);
+    close(fd);
     if (rc == MAP_FAILED) {
         perror("log mmap error");
     }
+
+    if (dev->log_table) {
+        munmap(dev->log_table, dev->log_size);
+    }
     dev->log_table = rc;
     dev->log_size = log_mmap_size;