[Libguestfs] [PATCH 3/4] appliance: Move code for creating supermin appliance directory to tmpdirs.c.

[Richard W.M. Jones]

This is largely code motion.
---
 src/appliance.c        | 40 +++++++-----------------------------
 src/guestfs-internal.h |  1 +
 src/tmpdirs.c          | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/src/appliance.c b/src/appliance.c
index 2cf6374..d293c2b 100644
--- a/src/appliance.c
+++ b/src/appliance.c
@@ -48,7 +48,7 @@ static int dir_contains_files (guestfs_h *g, const char *dir, ...);
 static int contains_old_style_appliance (guestfs_h *g, const char *path, void *data);
 static int contains_fixed_appliance (guestfs_h *g, const char *path, void *data);
 static int contains_supermin_appliance (guestfs_h *g, const char *path, void *data);
-static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, uid_t uid, char **kernel, char **initrd, char **appliance);
+static int build_supermin_appliance (guestfs_h *g, const char *supermin_path, char **kernel, char **initrd, char **appliance);
 static int run_supermin_build (guestfs_h *g, const char *lockfile, const char *appliancedir, const char *supermin_path);
 
 /**
@@ -133,7 +133,6 @@ build_appliance (guestfs_h *g,
                  char **appliance)
 {
   int r;
-  uid_t uid = geteuid ();
   CLEANUP_FREE char *supermin_path = NULL;
   CLEANUP_FREE char *path = NULL;
 
@@ -144,7 +143,7 @@ build_appliance (guestfs_h *g,
 
   if (r == 1)
     /* Step (2): build supermin appliance. */
-    return build_supermin_appliance (g, supermin_path, uid,
+    return build_supermin_appliance (g, supermin_path,
                                      kernel, initrd, appliance);
 
   /* Step (3). */
@@ -212,43 +211,18 @@ contains_supermin_appliance (guestfs_h *g, const char *path, void *data)
 static int
 build_supermin_appliance (guestfs_h *g,
                           const char *supermin_path,
-                          uid_t uid,
                           char **kernel, char **initrd,
                           char **appliance)
 {
-  CLEANUP_FREE char *tmpdir = guestfs_get_cachedir (g);
   CLEANUP_FREE char *cachedir = NULL, *lockfile = NULL, *appliancedir = NULL;
-  struct stat statbuf;
 
-  cachedir = safe_asprintf (g, "%s/.guestfs-%ju", tmpdir, (uintmax_t) uid);
-  lockfile = safe_asprintf (g, "%s/lock", cachedir);
+  cachedir = guestfs_int_lazy_make_supermin_appliance_dir (g);
+  if (cachedir == NULL)
+    return -1;
+
   appliancedir = safe_asprintf (g, "%s/appliance.d", cachedir);
+  lockfile = safe_asprintf (g, "%s/lock", cachedir);
 
-  ignore_value (mkdir (cachedir, 0755));
-  ignore_value (chmod (cachedir, 0755)); /* RHBZ#921292 */
-
-  /* See if the cache directory exists and passes some simple checks
-   * to make sure it has not been tampered with.
-   */
-  if (lstat (cachedir, &statbuf) == -1)
-    return 0;
-  if (statbuf.st_uid != uid) {
-    error (g, _("security: cached appliance %s is not owned by UID %ju"),
-           cachedir, (uintmax_t) uid);
-    return -1;
-  }
-  if (!S_ISDIR (statbuf.st_mode)) {
-    error (g, _("security: cached appliance %s is not a directory (mode %o)"),
-           cachedir, statbuf.st_mode);
-    return -1;
-  }
-  if ((statbuf.st_mode & 0022) != 0) {
-    error (g, _("security: cached appliance %s is writable by group or other (mode %o)"),
-           cachedir, statbuf.st_mode);
-    return -1;
-  }
-
-  (void) utimes (cachedir, NULL);
   debug (g, "begin building supermin appliance");
 
   /* Build the appliance if it needs to be built. */
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index dbd0a98..d325f50 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -758,6 +758,7 @@ extern int guestfs_int_set_env_tmpdir (guestfs_h *g, const char *tmpdir);
 extern int guestfs_int_set_env_runtimedir (guestfs_h *g, const char *runtimedir);
 extern int guestfs_int_lazy_make_tmpdir (guestfs_h *g);
 extern int guestfs_int_lazy_make_sockdir (guestfs_h *g);
+extern char *guestfs_int_lazy_make_supermin_appliance_dir (guestfs_h *g);
 extern void guestfs_int_remove_tmpdir (guestfs_h *g);
 extern void guestfs_int_remove_sockdir (guestfs_h *g);
 extern void guestfs_int_recursive_remove_dir (guestfs_h *g, const char *dir);
diff --git a/src/tmpdirs.c b/src/tmpdirs.c
index 293e4ea..6c8fe0d 100644
--- a/src/tmpdirs.c
+++ b/src/tmpdirs.c
@@ -213,6 +213,61 @@ guestfs_int_lazy_make_sockdir (guestfs_h *g)
 }
 
 /**
+ * Create the supermin appliance directory under cachedir, if it does
+ * not exist.
+ *
+ * Sanity-check that the permissions on the cachedir are safe, in case
+ * it has been pre-created maliciously or tampered with.
+ *
+ * Returns the directory name which the caller must free.
+ */
+char *
+guestfs_int_lazy_make_supermin_appliance_dir (guestfs_h *g)
+{
+  CLEANUP_FREE char *tmpdir = guestfs_get_cachedir (g);
+  char *ret = NULL;
+  struct stat statbuf;
+  uid_t uid = geteuid ();
+
+  ret = safe_asprintf (g, "%s/.guestfs-%ju", tmpdir, (uintmax_t) uid);
+
+  ignore_value (mkdir (ret, 0755));
+  ignore_value (chmod (ret, 0755)); /* RHBZ#921292 */
+
+  /* See if the cache directory exists and passes some simple checks
+   * to make sure it has not been tampered with.
+   */
+  if (lstat (ret, &statbuf) == -1) {
+    perrorf (g, _("stat: %s"), ret);
+    free (ret);
+    return NULL;
+  }
+  if (statbuf.st_uid != uid) {
+    error (g, _("security: cached appliance %s is not owned by UID %ju"),
+           ret, (uintmax_t) uid);
+    free (ret);
+    return NULL;
+  }
+  if (!S_ISDIR (statbuf.st_mode)) {
+    error (g, _("security: cached appliance %s is not a directory (mode %o)"),
+           ret, statbuf.st_mode);
+    free (ret);
+    return NULL;
+  }
+  if ((statbuf.st_mode & 0022) != 0) {
+    error (g, _("security: cached appliance %s is writable by group or other (mode %o)"),
+           ret, statbuf.st_mode);
+    free (ret);
+    return NULL;
+  }
+
+  /* "Touch" the directory. */
+  ignore_value (utimes (ret, NULL));
+
+  return ret;
+}
+
+/**
  * Recursively remove a temporary directory.  If removal fails, just
  * return (it's a temporary directory so it'll eventually be cleaned
  * up by a temp cleaner).
-- 
2.7.4