[PATCH] Fix race condition in qemuGetProcessInfo

[Eduardo Costa <eduardobmc@xxxxxxxxx>]

There is a race condition between the fopen and fscanf calls
in qemuGetProcessInfo. If fopen succeeds, there is a small
possibility that the file no longer exists before reading from it.
Now, if either fopen or fscanf calls fail, the function will behave
just as only fopen had failed.
---
 src/qemu/qemu_driver.c |   43 ++++++++++++++++---------------------------
 1 file changed, 16 insertions(+), 27 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ad5f70a..9ab1a81 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1310,9 +1310,9 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
 {
     char *proc;
     FILE *pidinfo;
-    unsigned long long usertime, systime;
-    long rss;
-    int cpu;
+    unsigned long long usertime = 0, systime = 0;
+    long rss = 0;
+    int cpu = 0;
     int ret;
 
     /* In general, we cannot assume pid_t fits in int; but /proc parsing
@@ -1324,33 +1324,24 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
     if (ret < 0)
         return -1;
 
-    if (!(pidinfo = fopen(proc, "r"))) {
-        /* VM probably shut down, so fake 0 */
-        if (cpuTime)
-            *cpuTime = 0;
-        if (lastCpu)
-            *lastCpu = 0;
-        if (vm_rss)
-            *vm_rss = 0;
-        VIR_FREE(proc);
-        return 0;
-    }
+    pidinfo = fopen(proc, "r");
     VIR_FREE(proc);
 
     /* See 'man proc' for information about what all these fields are. We're
      * only interested in a very few of them */
-    if (fscanf(pidinfo,
-               /* pid -> stime */
-               "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu"
-               /* cutime -> endcode */
-               "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
-               /* startstack -> processor */
-               "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
-               &usertime, &systime, &rss, &cpu) != 4) {
+    if (pidinfo) {
+        if (fscanf(pidinfo,
+                   /* pid -> stime */
+                   "%*d %*s %*c %*d %*d %*d %*d %*d %*u %*u %*u %*u %*u %llu %llu"
+                   /* cutime -> endcode */
+                   "%*d %*d %*d %*d %*d %*d %*u %*u %ld %*u %*u %*u"
+                   /* startstack -> processor */
+                   "%*u %*u %*u %*u %*u %*u %*u %*u %*u %*u %*d %d",
+                   &usertime, &systime, &rss, &cpu) != 4) {
+            VIR_WARN("cannot parse process status data");
+            usertime = systime = 0; rss = 0; cpu = 0;
+        }
         VIR_FORCE_FCLOSE(pidinfo);
-        VIR_WARN("cannot parse process status data");
-        errno = -EINVAL;
-        return -1;
     }
 
     /* We got jiffies
@@ -1374,8 +1365,6 @@ qemuGetProcessInfo(unsigned long long *cpuTime, int *lastCpu, long *vm_rss,
     VIR_DEBUG("Got status for %d/%d user=%llu sys=%llu cpu=%d rss=%ld",
               (int) pid, tid, usertime, systime, cpu, rss);
 
-    VIR_FORCE_FCLOSE(pidinfo);
-
     return 0;
 }
 
-- 
1.7.9.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list