[Qemu-devel] [PATCHv3 0/5] plug memory and file-descriptor leaks

[Qemu-devel] [PATCHv3 0/5] plug memory and file-descriptor leaks

[Jim Meyering]

From: Jim Meyering <meyering@redhat.com>

Hi Anthony,

I posted this series back in May, got some good feedback leading to a
pair of v2 patches.  Since then one of the 6 patches was applied.
I'm calling this v3, but it is merely a trivial rebase of the v1 and v2
patches.  Hoping it's not too late for 1.2, here are the remaining five:

Jim Meyering (5):
  qemu-ga: don't leak a file descriptor upon failed lockf
  linux-user: do_msgrcv: don't leak host_mb upon TARGET_EFAULT failure
  sheepdog: don't leak socket file descriptor upon connection failure
  arm-semi: don't leak 1KB user string lock buffer upon TARGET_SYS_OPEN
  softmmu-semi: fix lock_user* functions not to deref NULL upon OOM

 block/sheepdog.c      |  1 +
 linux-user/syscall.c  |  4 ++--
 qemu-ga.c             |  3 +++
 softmmu-semi.h        |  5 ++++-
 target-arm/arm-semi.c | 13 +++++++------
 5 files changed, 17 insertions(+), 9 deletions(-)

--
1.7.12

[Qemu-devel] [PATCHv3 1/5] qemu-ga: don't leak a file descriptor upon failed lockf

[Jim Meyering]

From: Jim Meyering <meyering@redhat.com>


Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 qemu-ga.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-ga.c b/qemu-ga.c
index 8f87621..26671fe 100644
--- a/qemu-ga.c
+++ b/qemu-ga.c
@@ -247,6 +247,9 @@ static bool ga_open_pidfile(const char *pidfile)
     pidfd = open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
     if (pidfd == -1 || lockf(pidfd, F_TLOCK, 0)) {
         g_critical("Cannot lock pid file, %s", strerror(errno));
+        if (pidfd != -1) {
+            close(pidfd);
+        }
         return false;
     }

-- 
1.7.12

[Qemu-devel] [PATCHv3 2/5] linux-user: do_msgrcv: don't leak host_mb upon TARGET_EFAULT failure

[Jim Meyering]

From: Jim Meyering <meyering@redhat.com>

Also, use g_malloc to avoid NULL-deref upon OOM.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 linux-user/syscall.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 41c869b..1174306 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -2848,7 +2848,7 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
     if (!lock_user_struct(VERIFY_WRITE, target_mb, msgp, 0))
         return -TARGET_EFAULT;

-    host_mb = malloc(msgsz+sizeof(long));
+    host_mb = g_malloc(msgsz+sizeof(long));
     ret = get_errno(msgrcv(msqid, host_mb, msgsz, tswapal(msgtyp), msgflg));

     if (ret > 0) {
@@ -2863,11 +2863,11 @@ static inline abi_long do_msgrcv(int msqid, abi_long msgp,
     }

     target_mb->mtype = tswapal(host_mb->mtype);
-    free(host_mb);

 end:
     if (target_mb)
         unlock_user_struct(target_mb, msgp, 1);
+    g_free(host_mb);
     return ret;
 }

-- 
1.7.12

[Qemu-devel] [PATCHv3 3/5] sheepdog: don't leak socket file descriptor upon connection failure

[Jim Meyering]

From: Jim Meyering <meyering@redhat.com>


Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 block/sheepdog.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index a04ad99..df4f441 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -485,6 +485,7 @@ static int connect_to_sdog(const char *addr, const char *port)
             if (errno == EINTR) {
                 goto reconnect;
             }
+            close(fd);
             break;
         }

-- 
1.7.12

[Qemu-devel] [PATCHv3 4/5] arm-semi: don't leak 1KB user string lock buffer upon TARGET_SYS_OPEN

[Jim Meyering]

From: Jim Meyering <meyering@redhat.com>

Always call unlock_user before returning.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 target-arm/arm-semi.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/target-arm/arm-semi.c b/target-arm/arm-semi.c
index 2495206..73bde58 100644
--- a/target-arm/arm-semi.c
+++ b/target-arm/arm-semi.c
@@ -194,18 +194,19 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         if (!(s = lock_user_string(ARG(0))))
             /* FIXME - should this error code be -TARGET_EFAULT ? */
             return (uint32_t)-1;
-        if (ARG(1) >= 12)
+        if (ARG(1) >= 12) {
+            unlock_user(s, ARG(0), 0);
             return (uint32_t)-1;
+        }
         if (strcmp(s, ":tt") == 0) {
-            if (ARG(1) < 4)
-                return STDIN_FILENO;
-            else
-                return STDOUT_FILENO;
+            int result_fileno = ARG(1) < 4 ? STDIN_FILENO : STDOUT_FILENO;
+            unlock_user(s, ARG(0), 0);
+            return result_fileno;
         }
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "open,%s,%x,1a4", ARG(0),
 			   (int)ARG(2)+1, gdb_open_modeflags[ARG(1)]);
-            return env->regs[0];
+            ret = env->regs[0];
         } else {
             ret = set_swi_errno(ts, open(s, open_modeflags[ARG(1)], 0644));
         }
-- 
1.7.12

[Qemu-devel] [PATCHv3 5/5] softmmu-semi: fix lock_user* functions not to deref NULL upon OOM

[Jim Meyering]

From: Jim Meyering <meyering@redhat.com>

Return NULL upon malloc failure.

Signed-off-by: Jim Meyering <meyering@redhat.com>
---
 softmmu-semi.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/softmmu-semi.h b/softmmu-semi.h
index 648cb95..bcb979a 100644
--- a/softmmu-semi.h
+++ b/softmmu-semi.h
@@ -40,7 +40,7 @@ static void *softmmu_lock_user(CPUArchState *env, uint32_t addr, uint32_t len,
     uint8_t *p;
     /* TODO: Make this something that isn't fixed size.  */
     p = malloc(len);
-    if (copy)
+    if (p && copy)
         cpu_memory_rw_debug(env, addr, p, len, 0);
     return p;
 }
@@ -52,6 +52,9 @@ static char *softmmu_lock_user_string(CPUArchState *env, uint32_t addr)
     uint8_t c;
     /* TODO: Make this something that isn't fixed size.  */
     s = p = malloc(1024);
+    if (!s) {
+        return NULL;
+    }
     do {
         cpu_memory_rw_debug(env, addr, &c, 1, 0);
         addr++;
-- 
1.7.12