[libglnx] dirfd: New tmpdir API

[Colin Walters <walters src gnome org>]

commit 7100ebbc6800c7e5e2e09cefe067fbb88b32f10b
Author: Colin Walters <walters verbum org>
Date:   Fri Aug 18 14:45:07 2017 -0400

    dirfd: New tmpdir API
    
    Basically all of the ostree/rpm-ostree callers want to both create and open, so
    let's merge `glnx_mkdtempat()` and `glnx_mkdtempat_open()`.
    
    Second, all of them want to do `glnx_shutil_rm_rf_at()` on cleanup, so we do the
    same thing we did with `GLnxTmpfile` and create `GLnxTmpDir` that has a cleanup
    attribute.
    
    The cleanup this results in for rpm-ostree is pretty substantial.

 glnx-dirfd.c                |  131 ++++++++++++++++++++++---------------------
 glnx-dirfd.h                |   25 ++++----
 tests/test-libglnx-xattrs.c |   17 ++---
 3 files changed, 87 insertions(+), 86 deletions(-)
---
diff --git a/glnx-dirfd.c b/glnx-dirfd.c
index 29a3ff7..b745e28 100644
--- a/glnx-dirfd.c
+++ b/glnx-dirfd.c
@@ -25,6 +25,7 @@
 #include <glnx-dirfd.h>
 #include <glnx-errors.h>
 #include <glnx-local-alloc.h>
+#include <glnx-shutil.h>
 
 /**
  * glnx_opendirat_with_errno:
@@ -283,27 +284,36 @@ glnx_gen_temp_name (gchar *tmpl)
 /**
  * glnx_mkdtempat:
  * @dfd: Directory fd
- * @tmpl: (type filename): template directory name, last 6 characters will be replaced
- * @mode: permissions to create the temporary directory with
+ * @tmpl: (type filename): Initial template directory name, last 6 characters will be replaced
+ * @mode: permissions with which to create the temporary directory
+ * @out_tmpdir: (out caller-allocates): Initialized tempdir structure
  * @error: Error
  *
- * Similar to g_mkdtemp_full, but using openat.
+ * Somewhat similar to g_mkdtemp_full(), but fd-relative, and returns a
+ * structure that uses autocleanups.  Note that the supplied @dfd lifetime
+ * must match or exceed that of @out_tmpdir in order to remove the directory.
  */
 gboolean
-glnx_mkdtempat (int dfd,
-                gchar *tmpl,
-                int mode,
-                GError **error)
+glnx_mkdtempat (int dfd, const char *tmpl, int mode,
+                GLnxTmpDir *out_tmpdir, GError **error)
 {
-  int count;
+  g_return_val_if_fail (tmpl != NULL, FALSE);
+  g_return_val_if_fail (out_tmpdir != NULL, FALSE);
+  g_return_val_if_fail (!out_tmpdir->initialized, FALSE);
 
-  g_return_val_if_fail (tmpl != NULL, -1);
+  dfd = glnx_dirfd_canonicalize (dfd);
 
-  for (count = 0; count < 100; count++)
+  g_autofree char *path = g_strdup (tmpl);
+  for (int count = 0; count < 100; count++)
     {
-      glnx_gen_temp_name (tmpl);
-
-      if (mkdirat (dfd, tmpl, mode) == -1)
+      glnx_gen_temp_name (path);
+
+      /* Ideally we could use openat(O_DIRECTORY | O_CREAT | O_EXCL) here
+       * to create and open the directory atomically, but that’s not supported by
+       * current kernel versions: http://www.openwall.com/lists/oss-security/2014/11/26/14
+       * (Tested on kernel 4.10.10-200.fc25.x86_64). For the moment, accept a
+       * TOCTTOU race here. */
+      if (mkdirat (dfd, path, mode) == -1)
         {
           if (errno == EEXIST)
             continue;
@@ -314,77 +324,72 @@ glnx_mkdtempat (int dfd,
           return glnx_throw_errno_prefix (error, "mkdirat");
         }
 
+      /* And open it */
+      glnx_fd_close int ret_dfd = -1;
+      if (!glnx_opendirat (dfd, path, FALSE, &ret_dfd, error))
+        {
+          /* If we fail to open, let's try to clean up */
+          (void)unlinkat (dfd, path, AT_REMOVEDIR);
+          return FALSE;
+        }
+
+      /* Return the initialized directory struct */
+      out_tmpdir->initialized = TRUE;
+      out_tmpdir->src_dfd = dfd; /* referenced; see above docs */
+      out_tmpdir->fd = glnx_steal_fd (&ret_dfd);
+      out_tmpdir->path = g_steal_pointer (&path);
       return TRUE;
     }
 
+  /* Failure */
   g_set_error (error, G_IO_ERROR, G_IO_ERROR_EXISTS,
-               "mkstempat ran out of combinations to try.");
+               "glnx_mkdtempat ran out of combinations to try");
   return FALSE;
 }
 
 /**
- * glnx_mkdtempat_open:
- * @dfd: Directory FD
- * @tmpl: (type filename): template directory name, last 6 characters will be replaced
+ * glnx_mkdtemp:
+ * @tmpl: (type filename): Source template directory name, last 6 characters will be replaced
  * @mode: permissions to create the temporary directory with
- * @out_dfd: (out caller-allocates): Return location for an FD for the new
- *   temporary directory, or `-1` on error
+ * @out_tmpdir: (out caller-allocates): Return location for tmpdir data
  * @error: Return location for a #GError, or %NULL
  *
- * Similar to glnx_mkdtempat(), except it will open the resulting temporary
- * directory and return a directory FD to it.
+ * Similar to glnx_mkdtempat(), but will use g_get_tmp_dir() as the parent
+ * directory to @tmpl.
  *
  * Returns: %TRUE on success, %FALSE otherwise
  * Since: UNRELEASED
  */
 gboolean
-glnx_mkdtempat_open (int      dfd,
-                     gchar   *tmpl,
-                     int      mode,
-                     int     *out_dfd,
-                     GError **error)
+glnx_mkdtemp (const gchar   *tmpl,
+              int      mode,
+              GLnxTmpDir *out_tmpdir,
+              GError **error)
 {
-  /* FIXME: Ideally we could use openat(O_DIRECTORY | O_CREAT | O_EXCL) here
-   * to create and open the directory atomically, but that’s not supported by
-   * current kernel versions: http://www.openwall.com/lists/oss-security/2014/11/26/14
-   * (Tested on kernel 4.10.10-200.fc25.x86_64). For the moment, accept a
-   * TOCTTOU race here. */
-  *out_dfd = -1;
-
-  if (!glnx_mkdtempat (dfd, tmpl, mode, error))
-    return FALSE;
-
-  return glnx_opendirat (dfd, tmpl, FALSE, out_dfd, error);
+  g_autofree char *path = g_build_filename (g_get_tmp_dir (), tmpl, NULL);
+  return glnx_mkdtempat (AT_FDCWD, path, mode,
+                         out_tmpdir, error);
 }
 
-/**
- * glnx_mkdtempat_open_in_system:
- * @tmpl: (type filename): template directory name, last 6 characters will be replaced
- * @mode: permissions to create the temporary directory with
- * @out_dfd: (out caller-allocates): Return location for an FD for the new
- *   temporary directory, or `-1` on error
- * @error: Return location for a #GError, or %NULL
- *
- * Similar to glnx_mkdtempat_open(), except it will use the system temporary
- * directory (from g_get_tmp_dir()) as the parent directory to @tmpl.
- *
- * Returns: %TRUE on success, %FALSE otherwise
- * Since: UNRELEASED
+/* Deallocate a tmpdir, closing the fd and (recursively) deleting the path. This
+ * is normally called by default by the autocleanup attribute, but you can also
+ * invoke this directly.
  */
-gboolean
-glnx_mkdtempat_open_in_system (gchar   *tmpl,
-                               int      mode,
-                               int     *out_dfd,
-                               GError **error)
+void
+glnx_tmpdir_clear (GLnxTmpDir *tmpd)
 {
-  glnx_fd_close int tmp_dfd = -1;
-
-  *out_dfd = -1;
-
-  if (!glnx_opendirat (-1, g_get_tmp_dir (), TRUE, &tmp_dfd, error))
-    return FALSE;
-
-  return glnx_mkdtempat_open (tmp_dfd, tmpl, mode, out_dfd, error);
+  /* Support being passed NULL so we work nicely in a GPtrArray */
+  if (!tmpd)
+    return;
+  if (!tmpd->initialized)
+    return;
+  g_assert_cmpint (tmpd->fd, !=, -1);
+  (void) close (tmpd->fd);
+  g_assert (tmpd->path);
+  g_assert_cmpint (tmpd->src_dfd, !=, -1);
+  (void) glnx_shutil_rm_rf_at (tmpd->src_dfd, tmpd->path, NULL, NULL);
+  g_free (tmpd->path);
+  tmpd->initialized = FALSE;
 }
 
 
diff --git a/glnx-dirfd.h b/glnx-dirfd.h
index 8e582fc..5e362fa 100644
--- a/glnx-dirfd.h
+++ b/glnx-dirfd.h
@@ -113,20 +113,19 @@ glnx_ensure_dir (int           dfd,
   return TRUE;
 }
 
-gboolean glnx_mkdtempat (int dfd,
-                         gchar *tmpl,
-                         int mode,
-                         GError **error);
+typedef struct {
+  gboolean initialized;
+  int src_dfd;
+  int fd;
+  char *path;
+} GLnxTmpDir;
+void glnx_tmpdir_clear (GLnxTmpDir *tmpf);
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(GLnxTmpDir, glnx_tmpdir_clear)
 
-gboolean glnx_mkdtempat_open (int      dfd,
-                              gchar   *tmpl,
-                              int      mode,
-                              int     *out_dfd,
-                              GError **error);
+gboolean glnx_mkdtempat (int dfd, const char *tmpl, int mode,
+                         GLnxTmpDir *out_tmpdir, GError **error);
 
-gboolean glnx_mkdtempat_open_in_system (gchar   *tmpl,
-                                        int      mode,
-                                        int     *out_dfd,
-                                        GError **error);
+gboolean glnx_mkdtemp (const char *tmpl, int      mode,
+                       GLnxTmpDir *out_tmpdir, GError **error);
 
 G_END_DECLS
diff --git a/tests/test-libglnx-xattrs.c b/tests/test-libglnx-xattrs.c
index 37c982d..1f94ce2 100644
--- a/tests/test-libglnx-xattrs.c
+++ b/tests/test-libglnx-xattrs.c
@@ -224,18 +224,17 @@ test_xattr_races (void)
   g_autoptr(GError) local_error = NULL;
   GError **error = &local_error;
   glnx_fd_close int dfd = -1;
-  g_autofree char *tmpdir = g_strdup_printf ("%s/libglnx-xattrs-XXXXXX",
-                                             getenv ("TMPDIR") ?: "/var/tmp");
+  g_auto(GLnxTmpDir) tmpdir = { 0, };
+  g_autofree char *tmpdir_path = g_strdup_printf ("%s/libglnx-xattrs-XXXXXX",
+                                                  getenv ("TMPDIR") ?: "/var/tmp");
   guint nread = 0;
 
-  if (!glnx_mkdtempat (AT_FDCWD, tmpdir, 0700, error))
-    goto out;
-
-  if (!glnx_opendirat (AT_FDCWD, tmpdir, TRUE, &dfd, error))
+  if (!glnx_mkdtempat (AT_FDCWD, tmpdir_path, 0700,
+                       &tmpdir, error))
     goto out;
 
   /* Support people building/testing on tmpfs https://github.com/flatpak/flatpak/issues/686 */
-  if (fsetxattr (dfd, "user.test", "novalue", strlen ("novalue"), 0) < 0)
+  if (fsetxattr (tmpdir.fd, "user.test", "novalue", strlen ("novalue"), 0) < 0)
     {
       if (errno == EOPNOTSUPP)
         {
@@ -252,7 +251,7 @@ test_xattr_races (void)
   for (guint i = 0; i < nprocs; i++)
     {
       struct XattrWorker *worker = &wdata[i];
-      worker->dfd = dfd;
+      worker->dfd = tmpdir.fd;
       worker->is_writer = i % 2 == 0;
       threads[i] = g_thread_new (NULL, xattr_thread, worker);
     }
@@ -267,8 +266,6 @@ test_xattr_races (void)
 
   g_print ("Read %u xattrs race free!\n", nread);
 
-  (void) glnx_shutil_rm_rf_at (AT_FDCWD, tmpdir, NULL, NULL);
-
  out:
   g_assert_no_error (local_error);
 }