[Qemu-devel] [PATCH v2 2/2] test-logging: don't hard-code paths in /tmp

[Sascha Silbe]

Since f6880b7f [qemu-log: support simple pid substitution for logs],
test-logging creates files with hard-coded names in /tmp. In the best
case, this prevents multiple developers from running "make check" on
the same machine. In the worst case, it allows for symlink attacks,
enabling an attacker to overwrite files that are writable to the
developer running "make check".

Instead of hard-coding the paths, create a temporary directory using
g_dir_make_tmp() and clean it up afterwards.

Fixes: f6880b7f ("qemu-log: support simple pid substitution for logs")
Signed-off-by: Sascha Silbe <address@hidden>
---
v1→v2:
  - Factor out g_build_filename() + qemu_set_log_filename() + g_free()
    into helper set_log_path_tmp().
  - Replace rmtree() spawning "rm -rf" with rmdir_full() using glib
    directory handling (non-recursive). Private to test-logging for
    now.

 tests/test-logging.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

diff --git a/tests/test-logging.c b/tests/test-logging.c
index cdf13c6..a12585f 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -25,6 +25,7 @@
  */
 
 #include "qemu/osdep.h"
+#include <glib/gstdio.h>
 
 #include "qemu-common.h"
 #include "qapi/error.h"
@@ -86,24 +87,57 @@ static void test_parse_range(void)
     error_free_or_abort(&err);
 }
 
-static void test_parse_path(void)
+static void set_log_path_tmp(char const *dir, char const *tpl, Error **errp)
 {
+    gchar *file_path = g_build_filename(dir, tpl, NULL);
+
+    qemu_set_log_filename(file_path, errp);
+    g_free(file_path);
+}
+
+static void test_parse_path(gconstpointer data)
+{
+    gchar const *tmp_path = data;
     Error *err = NULL;
 
-    qemu_set_log_filename("/tmp/qemu.log", &error_abort);
-    qemu_set_log_filename("/tmp/qemu-%d.log", &error_abort);
-    qemu_set_log_filename("/tmp/qemu.log.%d", &error_abort);
+    set_log_path_tmp(tmp_path, "qemu.log", &error_abort);
+    set_log_path_tmp(tmp_path, "qemu-%d.log", &error_abort);
+    set_log_path_tmp(tmp_path, "qemu.log.%d", &error_abort);
 
-    qemu_set_log_filename("/tmp/qemu-%d%d.log", &err);
+    set_log_path_tmp(tmp_path, "qemu-%d%d.log", &err);
     error_free_or_abort(&err);
 }
 
+/* Remove a directory and all its entries (non-recursive). */
+static void rmdir_full(gchar const *root)
+{
+    GDir *root_gdir = g_dir_open(root, 0, NULL);
+    gchar const *entry_name;
+
+    g_assert_nonnull(root_gdir);
+    while ((entry_name = g_dir_read_name(root_gdir)) != NULL) {
+        gchar *entry_path = g_build_filename(root, entry_name, NULL);
+        g_assert(g_remove(entry_path) == 0);
+        g_free(entry_path);
+    }
+    g_dir_close(root_gdir);
+    g_assert(g_rmdir(root) == 0);
+}
+
 int main(int argc, char **argv)
 {
+    gchar *tmp_path = g_dir_make_tmp("qemu-test-logging.XXXXXX", NULL);
+    int rc;
+
     g_test_init(&argc, &argv, NULL);
+    g_assert_nonnull(tmp_path);
 
     g_test_add_func("/logging/parse_range", test_parse_range);
-    g_test_add_func("/logging/parse_path", test_parse_path);
+    g_test_add_data_func("/logging/parse_path", tmp_path, test_parse_path);
 
-    return g_test_run();
+    rc = g_test_run();
+
+    rmdir_full(tmp_path);
+    g_free(tmp_path);
+    return rc;
 }
-- 
1.9.1