New upstream version 2017.12

libglnx/Makefile-libglnx.am

@@ -46,13 +46,14 @@ libglnx_la_SOURCES = \
 	$(libglnx_srcpath)/glnx-shutil.h \
 	$(libglnx_srcpath)/glnx-shutil.c \
 	$(libglnx_srcpath)/libglnx.h \
+	$(libglnx_srcpath)/tests/libglnx-testlib.h \
 	$(NULL)
 
 libglnx_la_CFLAGS = $(AM_CFLAGS) $(libglnx_cflags)
 libglnx_la_LDFLAGS = -avoid-version -Bsymbolic-functions -export-symbols-regex "^glnx_" -no-undefined -export-dynamic 
 libglnx_la_LIBADD = $(libglnx_libs)
 
-libglnx_tests = test-libglnx-xattrs test-libglnx-fdio test-libglnx-errors test-libglnx-macros
+libglnx_tests = test-libglnx-xattrs test-libglnx-fdio test-libglnx-errors test-libglnx-macros test-libglnx-shutil
 TESTS += $(libglnx_tests)
 
 check_PROGRAMS += $(libglnx_tests)
@@ -71,3 +72,7 @@ test_libglnx_errors_LDADD = $(libglnx_libs) libglnx.la
 test_libglnx_macros_SOURCES = $(libglnx_srcpath)/tests/test-libglnx-macros.c
 test_libglnx_macros_CFLAGS = $(AM_CFLAGS) $(libglnx_cflags)
 test_libglnx_macros_LDADD = $(libglnx_libs) libglnx.la
+
+test_libglnx_shutil_SOURCES = $(libglnx_srcpath)/tests/test-libglnx-shutil.c
+test_libglnx_shutil_CFLAGS = $(AM_CFLAGS) $(libglnx_cflags)
+test_libglnx_shutil_LDADD = $(libglnx_libs) libglnx.la

libglnx/Makefile-libglnx.am.inc

@@ -46,6 +46,7 @@ libglnx_la_SOURCES = \
 	libglnx/glnx-shutil.h \
 	libglnx/glnx-shutil.c \
 	libglnx/libglnx.h \
+	libglnx/tests/libglnx-testlib.h \
 	$(NULL)
 
 libglnx_la_CFLAGS = $(AM_CFLAGS) $(libglnx_cflags)

libglnx/glnx-dirfd.c

@@ -23,8 +23,10 @@
 #include <string.h>
 
 #include <glnx-dirfd.h>
+#include <glnx-fdio.h>
 #include <glnx-errors.h>
 #include <glnx-local-alloc.h>
+#include <glnx-shutil.h>
 
 /**
  * glnx_opendirat_with_errno:
@@ -203,8 +205,8 @@ glnx_dirfd_iterator_next_dent_ensure_dtype (GLnxDirFdIterator  *dfd_iter,
       if (ret_dent->d_type == DT_UNKNOWN)
         {
           struct stat stbuf;
-          if (TEMP_FAILURE_RETRY (fstatat (dfd_iter->fd, ret_dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW)) != 0)
-            return glnx_throw_errno (error);
+          if (!glnx_fstatat (dfd_iter->fd, ret_dent->d_name, &stbuf, AT_SYMLINK_NOFOLLOW, error))
+            return FALSE;
           ret_dent->d_type = IFTODT (stbuf.st_mode);
         }
     }
@@ -283,27 +285,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);
+      glnx_gen_temp_name (path);
 
-      if (mkdirat (dfd, tmpl, mode) == -1)
+      /* 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 +325,109 @@ 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;
+  g_autofree char *path = g_build_filename (g_get_tmp_dir (), tmpl, NULL);
+  return glnx_mkdtempat (AT_FDCWD, path, mode,
+                         out_tmpdir, error);
+}
 
-  if (!glnx_mkdtempat (dfd, tmpl, mode, error))
-    return FALSE;
-
-  return glnx_opendirat (dfd, tmpl, FALSE, out_dfd, error);
+static gboolean
+_glnx_tmpdir_free (GLnxTmpDir *tmpd,
+                   gboolean    delete_dir,
+                   GCancellable *cancellable,
+                   GError    **error)
+{
+  /* Support being passed NULL so we work nicely in a GPtrArray */
+  if (!(tmpd && tmpd->initialized))
+    return TRUE;
+  g_assert_cmpint (tmpd->fd, !=, -1);
+  (void) close (tmpd->fd);
+  tmpd->fd = -1;
+  g_assert (tmpd->path);
+  g_assert_cmpint (tmpd->src_dfd, !=, -1);
+  g_autofree char *path = tmpd->path; /* Take ownership */
+  tmpd->initialized = FALSE;
+  if (delete_dir)
+    {
+      if (!glnx_shutil_rm_rf_at (tmpd->src_dfd, path, cancellable, error))
+        return FALSE;
+    }
+  return TRUE;
 }
 
 /**
- * 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
+ * glnx_tmpdir_delete:
+ * @tmpf: Temporary dir
+ * @cancellable: Cancellable
+ * @error: Error
  *
- * Similar to glnx_mkdtempat_open(), except it will use the system temporary
- * directory (from g_get_tmp_dir()) as the parent directory to @tmpl.
+ * Deallocate a tmpdir, closing the fd and recursively deleting the path. This
+ * is normally called indirectly via glnx_tmpdir_cleanup() by the autocleanup
+ * attribute, but you can also invoke this directly.
  *
- * Returns: %TRUE on success, %FALSE otherwise
- * Since: UNRELEASED
+ * If an error occurs while deleting the filesystem path, @tmpf will still have
+ * been deallocated and should not be reused.
+ *
+ * See also `glnx_tmpdir_unset` to avoid deleting the path.
  */
 gboolean
-glnx_mkdtempat_open_in_system (gchar   *tmpl,
-                               int      mode,
-                               int     *out_dfd,
-                               GError **error)
+glnx_tmpdir_delete (GLnxTmpDir *tmpf, GCancellable *cancellable, GError **error)
 {
-  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);
+  return _glnx_tmpdir_free (tmpf, TRUE, cancellable, error);
 }
 
-
+/**
+ * glnx_tmpdir_unset:
+ * @tmpf: Temporary dir
+ * @cancellable: Cancellable
+ * @error: Error
+ *
+ * Deallocate a tmpdir, but do not delete the filesystem path.  See also
+ * `glnx_tmpdir_delete()`.
+ */
+void
+glnx_tmpdir_unset (GLnxTmpDir *tmpf)
+{
+  (void) _glnx_tmpdir_free (tmpf, FALSE, NULL, NULL);
+}

libglnx/glnx-dirfd.h

@@ -92,7 +92,7 @@ void glnx_gen_temp_name (gchar *tmpl);
  * @mode: Mode
  * @error: Return location for a #GError, or %NULL
  *
- * Wrapper around mkdirat() which ignores adds #GError support, ensures that
+ * Wrapper around mkdirat() which adds #GError support, ensures that
  * it retries on %EINTR, and also ignores `EEXIST`.
  *
  * See also `glnx_shutil_mkdir_p_at()` for recursive handling.
@@ -113,20 +113,25 @@ 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;
+gboolean glnx_tmpdir_delete (GLnxTmpDir *tmpf, GCancellable *cancellable, GError **error);
+void glnx_tmpdir_unset (GLnxTmpDir *tmpf);
+static inline void
+glnx_tmpdir_cleanup (GLnxTmpDir *tmpf)
+{
+  (void)glnx_tmpdir_delete (tmpf, NULL, NULL);
+}
+G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(GLnxTmpDir, glnx_tmpdir_cleanup)
 
-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

libglnx/glnx-fdio.c

@@ -112,7 +112,7 @@ rename_file_noreplace_at (int olddirfd, const char *oldpath,
           return TRUE;
         }
       else
-        return glnx_throw_errno (error);
+        return glnx_throw_errno_prefix (error, "renameat");
     }
   return TRUE;
 }
@@ -168,9 +168,11 @@ glnx_tmpfile_clear (GLnxTmpfile *tmpf)
     return;
   if (!tmpf->initialized)
     return;
-  if (tmpf->fd == -1)
-    return;
-  (void) close (tmpf->fd);
+  if (tmpf->fd != -1)
+    {
+      if (close (tmpf->fd) < 0)
+        g_assert (errno != EBADF);
+    }
   /* If ->path is set, we're likely aborting due to an error. Clean it up */
   if (tmpf->path)
     {
@@ -180,20 +182,11 @@ glnx_tmpfile_clear (GLnxTmpfile *tmpf)
   tmpf->initialized = FALSE;
 }
 
-/* Allocate a temporary file, using Linux O_TMPFILE if available. The file mode
- * will be 0600.
- *
- * The result will be stored in @out_tmpf, which is caller allocated
- * so you can store it on the stack in common scenarios.
- *
- * The directory fd @dfd must live at least as long as the output @out_tmpf.
- */
-gboolean
-glnx_open_tmpfile_linkable_at (int dfd,
-                               const char *subpath,
-                               int flags,
-                               GLnxTmpfile *out_tmpf,
-                               GError **error)
+static gboolean
+open_tmpfile_core (int dfd, const char *subpath,
+                   int flags,
+                   GLnxTmpfile *out_tmpf,
+                   GError **error)
 {
   const guint mode = 0600;
   glnx_fd_close int fd = -1;
@@ -201,9 +194,6 @@ glnx_open_tmpfile_linkable_at (int dfd,
 
   dfd = glnx_dirfd_canonicalize (dfd);
 
-  /* Don't allow O_EXCL, as that has a special meaning for O_TMPFILE */
-  g_return_val_if_fail ((flags & O_EXCL) == 0, FALSE);
-
   /* Creates a temporary file, that shall be renamed to "target"
    * later. If possible, this uses O_TMPFILE – in which case
    * "ret_path" will be returned as NULL. If not possible a the
@@ -260,6 +250,29 @@ glnx_open_tmpfile_linkable_at (int dfd,
   return FALSE;
 }
 
+/* Allocate a temporary file, using Linux O_TMPFILE if available. The file mode
+ * will be 0600.
+ *
+ * The result will be stored in @out_tmpf, which is caller allocated
+ * so you can store it on the stack in common scenarios.
+ *
+ * The directory fd @dfd must live at least as long as the output @out_tmpf.
+ */
+gboolean
+glnx_open_tmpfile_linkable_at (int dfd,
+                               const char *subpath,
+                               int flags,
+                               GLnxTmpfile *out_tmpf,
+                               GError **error)
+{
+  /* Don't allow O_EXCL, as that has a special meaning for O_TMPFILE;
+   * it's used for glnx_open_anonymous_tmpfile().
+   */
+  g_return_val_if_fail ((flags & O_EXCL) == 0, FALSE);
+
+  return open_tmpfile_core (dfd, subpath, flags, out_tmpf, error);
+}
+
 /* A variant of `glnx_open_tmpfile_linkable_at()` which doesn't support linking.
  * Useful for true temporary storage. The fd will be allocated in /var/tmp to
  * ensure maximum storage space.
@@ -269,7 +282,8 @@ glnx_open_anonymous_tmpfile (int          flags,
                              GLnxTmpfile *out_tmpf,
                              GError     **error)
 {
-  if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, "/var/tmp", flags, out_tmpf, error))
+  /* Add in O_EXCL */
+  if (!open_tmpfile_core (AT_FDCWD, "/var/tmp", flags | O_EXCL, out_tmpf, error))
     return FALSE;
   if (out_tmpf->path)
     {
@@ -428,8 +442,8 @@ glnx_fd_readall_malloc (int               fd,
   const guint maxreadlen = 4096;
 
   struct stat stbuf;
-  if (TEMP_FAILURE_RETRY (fstat (fd, &stbuf)) < 0)
-    return glnx_null_throw_errno (error);
+  if (!glnx_fstat (fd, &stbuf, error))
+    return FALSE;
 
   gsize buf_allocated;
   if (S_ISREG (stbuf.st_mode) && stbuf.st_size > 0)
@@ -597,7 +611,7 @@ glnx_readlinkat_malloc (int            dfd,
       c = g_malloc (l);
       n = TEMP_FAILURE_RETRY (readlinkat (dfd, subpath, c, l-1));
       if (n < 0)
-        return glnx_null_throw_errno (error);
+        return glnx_null_throw_errno_prefix (error, "readlinkat");
 
       if ((size_t) n < l-1)
         {
@@ -644,7 +658,7 @@ copy_symlink_at (int                   src_dfd,
   if (TEMP_FAILURE_RETRY (fchownat (dest_dfd, dest_subpath,
                                     src_stbuf->st_uid, src_stbuf->st_gid,
                                     AT_SYMLINK_NOFOLLOW)) != 0)
-    return glnx_throw_errno (error);
+    return glnx_throw_errno_prefix (error, "fchownat");
 
   return TRUE;
 }
@@ -852,17 +866,22 @@ glnx_regfile_copy_bytes (int fdf, int fdt, off_t max_bytes)
  * glnx_file_copy_at:
  * @src_dfd: Source directory fd
  * @src_subpath: Subpath relative to @src_dfd
+ * @src_stbuf: (allow-none): Optional stat buffer for source; if a stat() has already been done
  * @dest_dfd: Target directory fd
  * @dest_subpath: Destination name
  * @copyflags: Flags
  * @cancellable: cancellable
  * @error: Error
  *
- * Perform a full copy of the regular file or
- * symbolic link from @src_subpath to @dest_subpath.
+ * Perform a full copy of the regular file or symbolic link from @src_subpath to
+ * @dest_subpath; if @src_subpath is anything other than a regular file or
+ * symbolic link, an error will be returned.
  *
- * If @src_subpath is anything other than a regular
- * file or symbolic link, an error will be returned.
+ * If the source is a regular file and the destination exists as a symbolic
+ * link, the symbolic link will not be followed; rather the link itself will be
+ * replaced. Related to this: for regular files, when `GLNX_FILE_COPY_OVERWRITE`
+ * is specified, this function always uses `O_TMPFILE` (if available) and does a
+ * rename-into-place rather than `open(O_TRUNC)`.
  */
 gboolean
 glnx_file_copy_at (int                   src_dfd,
@@ -874,31 +893,23 @@ glnx_file_copy_at (int                   src_dfd,
                    GCancellable         *cancellable,
                    GError              **error)
 {
-  gboolean ret = FALSE;
-  int r;
-  int dest_open_flags;
-  struct timespec ts[2];
-  glnx_fd_close int src_fd = -1;
-  glnx_fd_close int dest_fd = -1;
-  struct stat local_stbuf;
-
-  if (g_cancellable_set_error_if_cancelled (cancellable, error))
-    goto out;
-
+  /* Canonicalize dfds */
   src_dfd = glnx_dirfd_canonicalize (src_dfd);
   dest_dfd = glnx_dirfd_canonicalize (dest_dfd);
 
+  if (g_cancellable_set_error_if_cancelled (cancellable, error))
+    return FALSE;
+
   /* Automatically do stat() if no stat buffer was supplied */
+  struct stat local_stbuf;
   if (!src_stbuf)
     {
-      if (fstatat (src_dfd, src_subpath, &local_stbuf, AT_SYMLINK_NOFOLLOW) != 0)
-        {
-          glnx_set_error_from_errno (error);
-          goto out;
-        }
+      if (!glnx_fstatat (src_dfd, src_subpath, &local_stbuf, AT_SYMLINK_NOFOLLOW, error))
+        return FALSE;
       src_stbuf = &local_stbuf;
     }
 
+  /* For symlinks, defer entirely to copy_symlink_at() */
   if (S_ISLNK (src_stbuf->st_mode))
     {
       return copy_symlink_at (src_dfd, src_subpath, src_stbuf,
@@ -910,47 +921,31 @@ glnx_file_copy_at (int                   src_dfd,
     {
       g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
                    "Cannot copy non-regular/non-symlink file: %s", src_subpath);
-      goto out;
+      return FALSE;
     }
 
+  /* Regular file path below here */
+
+  glnx_fd_close int src_fd = -1;
   if (!glnx_openat_rdonly (src_dfd, src_subpath, FALSE, &src_fd, error))
-    goto out;
+    return FALSE;
 
-  dest_open_flags = O_WRONLY | O_CREAT | O_CLOEXEC | O_NOCTTY;
-  if (!(copyflags & GLNX_FILE_COPY_OVERWRITE))
-    dest_open_flags |= O_EXCL;
-  else
-    dest_open_flags |= O_TRUNC;
+  /* Open a tmpfile for dest. Particularly for AT_FDCWD calls, we really want to
+   * open in the target directory, otherwise we may not be able to link.
+   */
+  g_auto(GLnxTmpfile) tmp_dest = { 0, };
+  { char *dnbuf = strdupa (dest_subpath);
+    const char *dn = dirname (dnbuf);
+    if (!glnx_open_tmpfile_linkable_at (dest_dfd, dn, O_WRONLY | O_CLOEXEC,
+                                        &tmp_dest, error))
+      return FALSE;
+  }
 
-  dest_fd = TEMP_FAILURE_RETRY (openat (dest_dfd, dest_subpath, dest_open_flags, src_stbuf->st_mode));
-  if (dest_fd == -1)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
+  if (glnx_regfile_copy_bytes (src_fd, tmp_dest.fd, (off_t) -1) < 0)
+    return glnx_throw_errno_prefix (error, "regfile copy");
 
-  r = glnx_regfile_copy_bytes (src_fd, dest_fd, (off_t) -1);
-  if (r < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-
-  if (fchown (dest_fd, src_stbuf->st_uid, src_stbuf->st_gid) != 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-
-  if (fchmod (dest_fd, src_stbuf->st_mode & 07777) != 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-
-  ts[0] = src_stbuf->st_atim;
-  ts[1] = src_stbuf->st_mtim;
-  (void) futimens (dest_fd, ts);
+  if (fchown (tmp_dest.fd, src_stbuf->st_uid, src_stbuf->st_gid) != 0)
+    return glnx_throw_errno_prefix (error, "fchown");
 
   if (!(copyflags & GLNX_FILE_COPY_NOXATTRS))
     {
@@ -958,35 +953,40 @@ glnx_file_copy_at (int                   src_dfd,
 
       if (!glnx_fd_get_all_xattrs (src_fd, &xattrs,
                                    cancellable, error))
-        goto out;
+        return FALSE;
 
-      if (!glnx_fd_set_all_xattrs (dest_fd, xattrs,
+      if (!glnx_fd_set_all_xattrs (tmp_dest.fd, xattrs,
                                    cancellable, error))
-        goto out;
+        return FALSE;
     }
 
+  /* Always chmod after setting xattrs, in case the file has mode 0400 or less,
+   * like /etc/shadow.  Linux currently allows write() on non-writable open files
+   * but not fsetxattr().
+   */
+  if (fchmod (tmp_dest.fd, src_stbuf->st_mode & 07777) != 0)
+    return glnx_throw_errno_prefix (error, "fchmod");
+
+  struct timespec ts[2];
+  ts[0] = src_stbuf->st_atim;
+  ts[1] = src_stbuf->st_mtim;
+  (void) futimens (tmp_dest.fd, ts);
+
   if (copyflags & GLNX_FILE_COPY_DATASYNC)
     {
-      if (fdatasync (dest_fd) < 0)
-        {
-          glnx_set_error_from_errno (error);
-          goto out;
-        }
-    }
-  
-  r = close (dest_fd);
-  dest_fd = -1;
-  if (r < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
+      if (fdatasync (tmp_dest.fd) < 0)
+        return glnx_throw_errno_prefix (error, "fdatasync");
     }
 
-  ret = TRUE;
- out:
-  if (!ret)
-    (void) unlinkat (dest_dfd, dest_subpath, 0);
-  return ret;
+  const GLnxLinkTmpfileReplaceMode replacemode =
+    (copyflags & GLNX_FILE_COPY_OVERWRITE) ?
+    GLNX_LINK_TMPFILE_REPLACE :
+    GLNX_LINK_TMPFILE_NOREPLACE;
+
+  if (!glnx_link_tmpfile_at (&tmp_dest, replacemode, dest_dfd, dest_subpath, error))
+    return FALSE;
+
+  return TRUE;
 }
 
 /**
@@ -1071,19 +1071,17 @@ glnx_file_replace_contents_with_perms_at (int                   dfd,
     return FALSE;
 
   if (glnx_loop_write (tmpf.fd, buf, len) < 0)
-    return glnx_throw_errno (error);
+    return glnx_throw_errno_prefix (error, "write");
 
   if (!(flags & GLNX_FILE_REPLACE_NODATASYNC))
     {
       struct stat stbuf;
       gboolean do_sync;
 
-      if (fstatat (dfd, subpath, &stbuf, AT_SYMLINK_NOFOLLOW) != 0)
-        {
-          if (errno != ENOENT)
-            return glnx_throw_errno (error);
-          do_sync = (flags & GLNX_FILE_REPLACE_DATASYNC_NEW) > 0;
-        }
+      if (!glnx_fstatat_allow_noent (dfd, subpath, &stbuf, AT_SYMLINK_NOFOLLOW, error))
+        return FALSE;
+      if (errno == ENOENT)
+        do_sync = (flags & GLNX_FILE_REPLACE_DATASYNC_NEW) > 0;
       else
         do_sync = TRUE;
 
@@ -1097,11 +1095,11 @@ glnx_file_replace_contents_with_perms_at (int                   dfd,
   if (uid != (uid_t) -1)
     {
       if (fchown (tmpf.fd, uid, gid) != 0)
-        return glnx_throw_errno (error);
+        return glnx_throw_errno_prefix (error, "fchown");
     }
 
   if (fchmod (tmpf.fd, mode) != 0)
-    return glnx_throw_errno (error);
+    return glnx_throw_errno_prefix (error, "fchmod");
 
   if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE,
                              dfd, subpath, error))

libglnx/glnx-fdio.h

@@ -295,6 +295,44 @@ glnx_fstatat (int           dfd,
   return TRUE;
 }
 
+/**
+ * glnx_fstatat_allow_noent:
+ * @dfd: Directory FD to stat beneath
+ * @path: Path to stat beneath @dfd
+ * @buf: (out caller-allocates): Return location for stat details
+ * @flags: Flags to pass to fstatat()
+ * @error: Return location for a #GError, or %NULL
+ *
+ * Like glnx_fstatat(), but handles `ENOENT` in a non-error way.  Instead,
+ * on success `errno` will be zero, otherwise it will be preserved.  Hence
+ * you can test `if (errno == 0)` to conditionalize on the file existing,
+ * or `if (errno == ENOENT)` for non-existence.
+ *
+ * Returns: %TRUE on success, %FALSE otherwise (errno is preserved)
+ * Since: UNRELEASED
+ */
+static inline gboolean
+glnx_fstatat_allow_noent (int               dfd,
+                          const char       *path,
+                          struct stat      *out_buf,
+                          int               flags,
+                          GError          **error)
+{
+  if (TEMP_FAILURE_RETRY (fstatat (dfd, path, out_buf, flags)) != 0)
+    {
+      if (errno != ENOENT)
+        {
+          int errsv = errno;
+          (void) glnx_throw_errno_prefix (error, "fstatat(%s)", path);
+          errno = errsv;
+          return FALSE;
+        }
+    }
+  else
+    errno = 0;
+  return TRUE;
+}
+
 /**
  * glnx_renameat:
  *

libglnx/glnx-lockfile.c

@@ -105,7 +105,7 @@ glnx_make_lock_file(int dfd, const char *p, int operation, GLnxLockFile *out_loc
                                 r = flock(fd, operation);
 
                         if (r < 0)
-                                return glnx_throw_errno(error);
+                                return glnx_throw_errno_prefix (error, "flock");
                 }
 
                 /* If we acquired the lock, let's check if the file
@@ -114,9 +114,8 @@ glnx_make_lock_file(int dfd, const char *p, int operation, GLnxLockFile *out_loc
                  * it. In such a case our acquired lock is worthless,
                  * hence try again. */
 
-                r = fstat(fd, &st);
-                if (r < 0)
-                        return glnx_throw_errno(error);
+                if (!glnx_fstat (fd, &st, error))
+                        return FALSE;
                 if (st.st_nlink > 0)
                         break;
 
@@ -127,21 +126,18 @@ glnx_make_lock_file(int dfd, const char *p, int operation, GLnxLockFile *out_loc
         /* Note that if this is not AT_FDCWD, the caller takes responsibility
          * for the fd's lifetime being >= that of the lock.
          */
+        out_lock->initialized = TRUE;
         out_lock->dfd = dfd;
-        out_lock->path = t;
-        out_lock->fd = fd;
+        out_lock->path = g_steal_pointer (&t);
+        out_lock->fd = glnx_steal_fd (&fd);
         out_lock->operation = operation;
-
-        fd = -1;
-        t = NULL;
-
         return TRUE;
 }
 
 void glnx_release_lock_file(GLnxLockFile *f) {
         int r;
 
-        if (!f)
+        if (!(f && f->initialized))
                 return;
 
         if (f->path) {
@@ -182,4 +178,5 @@ void glnx_release_lock_file(GLnxLockFile *f) {
                 (void) close (f->fd);
         f->fd = -1;
         f->operation = 0;
+        f->initialized = FALSE;
 }

libglnx/glnx-lockfile.h

@@ -27,6 +27,7 @@
 #include "glnx-backport-autoptr.h"
 
 typedef struct GLnxLockFile {
+        gboolean initialized;
         int dfd;
         char *path;
         int fd;
@@ -37,5 +38,3 @@ gboolean glnx_make_lock_file(int dfd, const char *p, int operation, GLnxLockFile
 void glnx_release_lock_file(GLnxLockFile *f);
 
 G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(GLnxLockFile, glnx_release_lock_file)
-
-#define GLNX_LOCK_FILE_INIT { .fd = -1, .dfd = AT_FDCWD, .path = NULL }

libglnx/glnx-macros.h

@@ -38,7 +38,7 @@ G_BEGIN_DECLS
         ({                                                              \
                 const char *_appendees_[] = { a, __VA_ARGS__ };         \
                 char *_d_, *_p_;                                        \
-                int _len_ = 0;                                          \
+                size_t _len_ = 0;                                       \
                 unsigned _i_;                                           \
                 for (_i_ = 0; _i_ < G_N_ELEMENTS(_appendees_) && _appendees_[_i_]; _i_++) \
                         _len_ += strlen(_appendees_[_i_]);              \

libglnx/glnx-shutil.c

@@ -148,7 +148,13 @@ mkdir_p_at_internal (int              dfd,
           g_assert (!did_recurse);
 
           lastslash = strrchr (path, '/');
-          g_assert (lastslash != NULL);
+          if (lastslash == NULL)
+            {
+              /* This can happen if @dfd was deleted between being opened and
+               * passed to mkdir_p_at_internal(). */
+              return glnx_throw_errno_prefix (error, "mkdir(%s)", path);
+            }
+
           /* Note we can mutate the buffer as we dup'd it */
           *lastslash = '\0';
 
@@ -187,6 +193,9 @@ mkdir_p_at_internal (int              dfd,
  * directory fd @dfd.
  *
  * See also glnx_ensure_dir() for a non-recursive version.
+ *
+ * This will return %G_IO_ERROR_NOT_FOUND if @dfd has been deleted since being
+ * opened. It may return other errors from mkdirat() in other situations.
  */
 gboolean
 glnx_shutil_mkdir_p_at (int                   dfd,

libglnx/tests/libglnx-testlib.h

@@ -0,0 +1,34 @@
+/* -*- mode: C; c-file-style: "gnu"; indent-tabs-mode: nil; -*-
+ *
+ * Copyright (C) 2017 Red Hat, Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the
+ * Free Software Foundation, Inc., 59 Temple Place - Suite 330,
+ * Boston, MA 02111-1307, USA.
+ */
+
+#pragma once
+
+typedef GError _GLnxTestAutoError;
+static inline void
+_glnx_test_auto_error_cleanup (_GLnxTestAutoError *autoerror)
+{
+  g_assert_no_error (autoerror);
+  /* We could add a clear call here, but no point...we'll have aborted */
+}
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(_GLnxTestAutoError, _glnx_test_auto_error_cleanup);
+
+#define _GLNX_TEST_DECLARE_ERROR(local_error, error)      \
+  g_autoptr(_GLnxTestAutoError) local_error = NULL; \
+  GError **error = &local_error

libglnx/tests/test-libglnx-fdio.c

@@ -26,6 +26,8 @@
 #include <err.h>
 #include <string.h>
 
+#include "libglnx-testlib.h"
+
 static gboolean
 renameat_test_setup (int *out_srcfd, int *out_destfd,
                      GError **error)
@@ -59,14 +61,13 @@ renameat_test_setup (int *out_srcfd, int *out_destfd,
 static void
 test_renameat2_noreplace (void)
 {
-  g_autoptr(GError) local_error = NULL;
-  GError **error = &local_error;
+  _GLNX_TEST_DECLARE_ERROR(local_error, error);
   glnx_fd_close int srcfd = -1;
   glnx_fd_close int destfd = -1;
   struct stat stbuf;
 
   if (!renameat_test_setup (&srcfd, &destfd, error))
-    goto out;
+    return;
 
   if (glnx_renameat2_noreplace (srcfd, "foo", destfd, "bar") == 0)
     g_assert_not_reached ();
@@ -76,117 +77,153 @@ test_renameat2_noreplace (void)
     }
 
   if (glnx_renameat2_noreplace (srcfd, "foo", destfd, "baz") < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
+    return (void)glnx_throw_errno_prefix (error, "renameat");
   if (!glnx_fstatat (destfd, "bar", &stbuf, AT_SYMLINK_NOFOLLOW, error))
-    goto out;
+    return;
 
   if (fstatat (srcfd, "foo", &stbuf, AT_SYMLINK_NOFOLLOW) == 0)
     g_assert_not_reached ();
   else
     g_assert_cmpint (errno, ==, ENOENT);
-
- out:
-  g_assert_no_error (local_error);
 }
 
 static void
 test_renameat2_exchange (void)
 {
-  g_autoptr(GError) local_error = NULL;
-  GError **error = &local_error;
+  _GLNX_TEST_DECLARE_ERROR(local_error, error);
+
   glnx_fd_close int srcfd = -1;
   glnx_fd_close int destfd = -1;
-  struct stat stbuf;
-
   if (!renameat_test_setup (&srcfd, &destfd, error))
-    goto out;
+    return;
 
   if (glnx_renameat2_exchange (AT_FDCWD, "srcdir", AT_FDCWD, "destdir") < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
+    return (void)glnx_throw_errno_prefix (error, "renameat");
 
   /* Ensure the dir fds are the same */
-  if (fstatat (srcfd, "foo", &stbuf, AT_SYMLINK_NOFOLLOW) < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-  if (fstatat (destfd, "bar", &stbuf, AT_SYMLINK_NOFOLLOW) < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
+  struct stat stbuf;
+  if (!glnx_fstatat (srcfd, "foo", &stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return;
+  if (!glnx_fstatat (destfd, "bar", &stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return;
   /* But the dirs should be swapped */
-  if (fstatat (AT_FDCWD, "destdir/foo", &stbuf, AT_SYMLINK_NOFOLLOW) < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-  if (fstatat (AT_FDCWD, "srcdir/bar", &stbuf, AT_SYMLINK_NOFOLLOW) < 0)
-    {
-      glnx_set_error_from_errno (error);
-      goto out;
-    }
-
- out:
-  g_assert_no_error (local_error);
+  if (!glnx_fstatat (AT_FDCWD, "destdir/foo", &stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return;
+  if (!glnx_fstatat (AT_FDCWD, "srcdir/bar", &stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return;
 }
 
 static void
 test_tmpfile (void)
 {
-  g_autoptr(GError) local_error = NULL;
-  GError **error = &local_error;
+  _GLNX_TEST_DECLARE_ERROR(local_error, error);
+
   g_auto(GLnxTmpfile) tmpf = { 0, };
-
   if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, ".", O_WRONLY|O_CLOEXEC, &tmpf, error))
-    goto out;
-
+    return;
   if (glnx_loop_write (tmpf.fd, "foo", strlen ("foo")) < 0)
-    {
-      (void)glnx_throw_errno_prefix (error, "write");
-      goto out;
-    }
-
+    return (void)glnx_throw_errno_prefix (error, "write");
   if (glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE, AT_FDCWD, "foo", error))
-    goto out;
-
- out:
-  g_assert_no_error (local_error);
+    return;
 }
 
 static void
 test_stdio_file (void)
 {
-  g_autoptr(GError) local_error = NULL;
-  GError **error = &local_error;
+  _GLNX_TEST_DECLARE_ERROR(local_error, error);
   g_auto(GLnxTmpfile) tmpf = { 0, };
   g_autoptr(FILE) f = NULL;
 
   if (!glnx_open_anonymous_tmpfile (O_RDWR|O_CLOEXEC, &tmpf, error))
-    goto out;
+    return;
   f = fdopen (tmpf.fd, "w");
+  tmpf.fd = -1; /* Ownership was transferred via fdopen() */
   if (!f)
-    {
-      (void)glnx_throw_errno_prefix (error, "fdopen");
-      goto out;
-    }
-
+    return (void)glnx_throw_errno_prefix (error, "fdopen");
   if (fwrite ("hello", 1, strlen ("hello"), f) != strlen ("hello"))
-    {
-      (void)glnx_throw_errno_prefix (error, "fwrite");
-      goto out;
-    }
+    return (void)glnx_throw_errno_prefix (error, "fwrite");
   if (!glnx_stdio_file_flush (f, error))
-    goto out;
+    return;
+}
 
- out:
+static void
+test_fstatat (void)
+{
+  _GLNX_TEST_DECLARE_ERROR(local_error, error);
+  struct stat stbuf = { 0, };
+
+  if (!glnx_fstatat_allow_noent (AT_FDCWD, ".", &stbuf, 0, error))
+    return;
+  g_assert_cmpint (errno, ==, 0);
   g_assert_no_error (local_error);
+  g_assert (S_ISDIR (stbuf.st_mode));
+  if (!glnx_fstatat_allow_noent (AT_FDCWD, "nosuchfile", &stbuf, 0, error))
+    return;
+  g_assert_cmpint (errno, ==, ENOENT);
+  g_assert_no_error (local_error);
+}
+
+static void
+test_filecopy (void)
+{
+  _GLNX_TEST_DECLARE_ERROR(local_error, error);
+  g_auto(GLnxTmpfile) tmpf = { 0, };
+  const char foo[] = "foo";
+  struct stat stbuf;
+
+  if (!glnx_ensure_dir (AT_FDCWD, "subdir", 0755, error))
+    return;
+
+  if (!glnx_file_replace_contents_at (AT_FDCWD, foo, (guint8*)foo, sizeof (foo),
+                                      GLNX_FILE_REPLACE_NODATASYNC, NULL, error))
+    return;
+
+  /* Copy it into both the same dir and a subdir */
+  if (!glnx_file_copy_at (AT_FDCWD, foo, NULL, AT_FDCWD, "bar",
+                          GLNX_FILE_COPY_NOXATTRS, NULL, error))
+    return;
+  if (!glnx_file_copy_at (AT_FDCWD, foo, NULL, AT_FDCWD, "subdir/bar",
+                          GLNX_FILE_COPY_NOXATTRS, NULL, error))
+    return;
+  if (!glnx_fstatat (AT_FDCWD, "subdir/bar", &stbuf, 0, error))
+    return;
+
+  if (glnx_file_copy_at (AT_FDCWD, foo, NULL, AT_FDCWD, "bar",
+                         GLNX_FILE_COPY_NOXATTRS, NULL, error))
+    g_assert_not_reached ();
+  g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_EXISTS);
+  g_clear_error (&local_error);
+
+  if (!glnx_file_copy_at (AT_FDCWD, foo, NULL, AT_FDCWD, "bar",
+                          GLNX_FILE_COPY_NOXATTRS | GLNX_FILE_COPY_OVERWRITE,
+                          NULL, error))
+    return;
+
+  if (symlinkat ("nosuchtarget", AT_FDCWD, "link") < 0)
+    return (void) glnx_throw_errno_prefix (error, "symlinkat");
+
+  /* Shouldn't be able to overwrite a symlink without GLNX_FILE_COPY_OVERWRITE */
+  if (glnx_file_copy_at (AT_FDCWD, foo, NULL, AT_FDCWD, "link",
+                         GLNX_FILE_COPY_NOXATTRS,
+                         NULL, error))
+    g_assert_not_reached ();
+  g_assert_error (local_error, G_IO_ERROR, G_IO_ERROR_EXISTS);
+  g_clear_error (&local_error);
+
+  /* Test overwriting symlink */
+  if (!glnx_file_copy_at (AT_FDCWD, foo, NULL, AT_FDCWD, "link",
+                          GLNX_FILE_COPY_NOXATTRS | GLNX_FILE_COPY_OVERWRITE,
+                          NULL, error))
+    return;
+
+  if (!glnx_fstatat_allow_noent (AT_FDCWD, "nosuchtarget", &stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return;
+  g_assert_cmpint (errno, ==, ENOENT);
+  g_assert_no_error (local_error);
+
+  if (!glnx_fstatat (AT_FDCWD, "link", &stbuf, AT_SYMLINK_NOFOLLOW, error))
+    return;
+  g_assert (S_ISREG (stbuf.st_mode));
 }
 
 int main (int argc, char **argv)
@@ -197,8 +234,10 @@ int main (int argc, char **argv)
 
   g_test_add_func ("/tmpfile", test_tmpfile);
   g_test_add_func ("/stdio-file", test_stdio_file);
+  g_test_add_func ("/filecopy", test_filecopy);
   g_test_add_func ("/renameat2-noreplace", test_renameat2_noreplace);
   g_test_add_func ("/renameat2-exchange", test_renameat2_exchange);
+  g_test_add_func ("/fstat", test_fstatat);
 
   ret = g_test_run();
 

libglnx/tests/test-libglnx-xattrs.c

@@ -223,19 +223,17 @@ test_xattr_races (void)
   GThread *threads[nprocs];
   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 +250,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 +265,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);
 }