Port to GLnxTmpfile

There's lots of mechanically replacing `OtTmpFile` with `GLnxTmpfile`;
the biggest changes are in the commit path.  Symlink commits are now
very clearly separated from regular files.  Symlinks are `OtCleanupUnlinkat`,
and regular files are `GLnxTmpfile`.

The commit codepath separates those as `_ostree_repo_commit_path_final()` and
`_ostree_repo_commit_tmpf_final()`. A nice aspect of all of this is that they
both *consume* the temporary on success. This avoids an extra spurious
`unlink()` call.

One of the biggest bits of code motion is in `commit_loose_regfile_object()`,
which no longer needs to care about symlinks. For the most parth though it's
just removing conditionals.

Update submodule: libglnx

Closes: #958
Approved by: jlebon

libglnx

@@ -1 +1 @@
-Subproject commit 32231fdb5273dd2a812c61549d6c0e681c0f5d59
+Subproject commit caa51ac24ffcdffcb610bc6ccc9da964d4be74ee

src/libostree/ostree-fetcher-curl.c

@@ -99,7 +99,7 @@ struct FetcherRequest {
   OstreeFetcherRequestFlags flags;
   gboolean is_membuf;
   GError *caught_write_error;
-  OtTmpfile tmpf;
+  GLnxTmpfile tmpf;
   GString *output_buf;
 
   CURL *easy;
@@ -270,7 +270,7 @@ ensure_tmpfile (FetcherRequest *req, GError **error)
 {
   if (!req->tmpf.initialized)
     {
-      if (!ot_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
+      if (!glnx_open_tmpfile_linkable_at (req->fetcher->tmpdir_dfd, ".",
                                           O_WRONLY | O_CLOEXEC, &req->tmpf,
                                           error))
         return FALSE;
@@ -390,7 +390,7 @@ check_multi_info (OstreeFetcher *fetcher)
                   glnx_set_error_from_errno (error);
                   g_task_return_error (task, g_steal_pointer (&local_error));
                 }
-              else if (!ot_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE,
+              else if (!glnx_link_tmpfile_at (&req->tmpf, GLNX_LINK_TMPFILE_REPLACE,
                                               fetcher->tmpdir_dfd, tmpfile_path,
                                               error))
                 g_task_return_error (task, g_steal_pointer (&local_error));
@@ -616,7 +616,7 @@ request_unref (FetcherRequest *req)
   g_ptr_array_unref (req->mirrorlist);
   g_free (req->filename);
   g_clear_error (&req->caught_write_error);
-  ot_tmpfile_clear (&req->tmpf);
+  glnx_tmpfile_clear (&req->tmpf);
   if (req->output_buf)
     g_string_free (req->output_buf, TRUE);
   curl_easy_cleanup (req->easy);

src/libostree/ostree-impl-system-generator.c

@@ -172,12 +172,11 @@ _ostree_impl_system_generator (const char *ostree_cmdline,
   /* Generate our bind mount unit */
   const char *stateroot_var_path = glnx_strjoina ("/sysroot/ostree/deploy/", stateroot, "/var");
 
-  glnx_fd_close int tmpfd = -1;
+  g_auto(GLnxTmpfile) tmpf = { 0, };
-  g_autofree char *tmppath = NULL;
   if (!glnx_open_tmpfile_linkable_at (normal_dir_dfd, ".", O_WRONLY | O_CLOEXEC,
-                                      &tmpfd, &tmppath, error))
+                                      &tmpf, error))
     return FALSE;
-  g_autoptr(GOutputStream) outstream = g_unix_output_stream_new (tmpfd, FALSE);
+  g_autoptr(GOutputStream) outstream = g_unix_output_stream_new (tmpf.fd, FALSE);
   gsize bytes_written;
   /* This code is inspired by systemd's fstab-generator.c.
    *
@@ -204,12 +203,11 @@ _ostree_impl_system_generator (const char *ostree_cmdline,
     return FALSE;
   g_clear_object (&outstream);
   /* It should be readable */
-  if (fchmod (tmpfd, 0644) < 0)
+  if (fchmod (tmpf.fd, 0644) < 0)
     return glnx_throw_errno_prefix (error, "fchmod");
   /* Error out if somehow it already exists, that'll help us debug conflicts */
-  if (!glnx_link_tmpfile_at (normal_dir_dfd, GLNX_LINK_TMPFILE_NOREPLACE,
+  if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE,
-                             tmpfd, tmppath, normal_dir_dfd,
+                             normal_dir_dfd, "var.mount", error))
-                             "var.mount", error))
     return FALSE;
 
   /* And ensure it's required; newer systemd will auto-inject fs dependencies

src/libostree/ostree-repo-checkout.c

@@ -60,8 +60,8 @@ checkout_object_for_uncompressed_cache (OstreeRepo      *self,
   guint32 file_mode = g_file_info_get_attribute_uint32 (src_info, "unix::mode");
   file_mode &= ~(S_ISUID|S_ISGID);
 
-  g_auto(OtTmpfile) tmpf = { 0, };
+  g_auto(GLnxTmpfile) tmpf = { 0, };
-  if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC,
+  if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY | O_CLOEXEC,
                                       &tmpf, error))
     return FALSE;
   g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);
@@ -89,7 +89,7 @@ checkout_object_for_uncompressed_cache (OstreeRepo      *self,
                                             cancellable, error))
     return FALSE;
 
-  if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
+  if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
                              self->uncompressed_objects_dir_fd, loose_path,
                              error))
     return FALSE;
@@ -254,10 +254,10 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
     }
   else if (g_file_info_get_file_type (file_info) == G_FILE_TYPE_REGULAR)
     {
-      g_auto(OtTmpfile) tmpf = { 0, };
+      g_auto(GLnxTmpfile) tmpf = { 0, };
       GLnxLinkTmpfileReplaceMode replace_mode;
 
-      if (!ot_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
+      if (!glnx_open_tmpfile_linkable_at (destination_dfd, ".", O_WRONLY | O_CLOEXEC,
                                           &tmpf, error))
         return FALSE;
 
@@ -285,7 +285,7 @@ create_file_copy_from_input_at (OstreeRepo     *repo,
       else
         replace_mode = GLNX_LINK_TMPFILE_NOREPLACE;
 
-      if (!ot_link_tmpfile_at (&tmpf, replace_mode,
+      if (!glnx_link_tmpfile_at (&tmpf, replace_mode,
                                  destination_dfd, destination_name,
                                  error))
         return FALSE;

src/libostree/ostree-repo-commit.c

@@ -134,21 +134,19 @@ ot_security_smack_reset_fd (int fd)
 #endif
 }
 
+/* Given an O_TMPFILE regular file, link it into place. */
 gboolean
-_ostree_repo_commit_loose_final (OstreeRepo        *self,
+_ostree_repo_commit_tmpf_final (OstreeRepo        *self,
                                 const char        *checksum,
                                 OstreeObjectType   objtype,
-                                 int                temp_dfd,
+                                GLnxTmpfile       *tmpf,
-                                 int                fd,
-                                 const char        *temp_filename,
                                 GCancellable      *cancellable,
                                 GError           **error)
 {
-  int dest_dfd;
   char tmpbuf[_OSTREE_LOOSE_PATH_MAX];
-
   _ostree_loose_path (tmpbuf, checksum, objtype, self->mode);
 
+  int dest_dfd;
   if (self->in_transaction)
     dest_dfd = self->commit_stagedir_fd;
   else
@@ -158,41 +156,64 @@ _ostree_repo_commit_loose_final (OstreeRepo        *self,
                                             cancellable, error))
     return FALSE;
 
-  if (fd != -1)
+  return glnx_link_tmpfile_at (tmpf, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
-    {
+                               dest_dfd, tmpbuf, error);
-      if (!glnx_link_tmpfile_at (temp_dfd, GLNX_LINK_TMPFILE_NOREPLACE_IGNORE_EXIST,
-                                 fd, temp_filename, dest_dfd, tmpbuf, error))
-        return FALSE;
 }
-  else
+
+/* Given a dfd+path combination (may be regular file or symlink),
+ * rename it into place.
+ */
+gboolean
+_ostree_repo_commit_path_final (OstreeRepo        *self,
+                                const char        *checksum,
+                                OstreeObjectType   objtype,
+                                OtCleanupUnlinkat *tmp_path,
+                                GCancellable      *cancellable,
+                                GError           **error)
 {
-      if (G_UNLIKELY (renameat (temp_dfd, temp_filename,
+  /* The final renameat() */
-                                dest_dfd, tmpbuf) == -1))
+  char tmpbuf[_OSTREE_LOOSE_PATH_MAX];
+  _ostree_loose_path (tmpbuf, checksum, objtype, self->mode);
+
+  int dest_dfd;
+  if (self->in_transaction)
+    dest_dfd = self->commit_stagedir_fd;
+  else
+    dest_dfd = self->objects_dir_fd;
+
+  if (!_ostree_repo_ensure_loose_objdir_at (dest_dfd, tmpbuf,
+                                            cancellable, error))
+    return FALSE;
+
+  if (renameat (tmp_path->dfd, tmp_path->path,
+                dest_dfd, tmpbuf) == -1)
     {
       if (errno != EEXIST)
-            return glnx_throw_errno_prefix (error, "Storing file '%s'", temp_filename);
+        return glnx_throw_errno_prefix (error, "Storing file '%s'", tmp_path->path);
-          else
+      /* Otherwise, the caller's cleanup will unlink+free */
-            (void) unlinkat (temp_dfd, temp_filename, 0);
     }
+  else
+    {
+      /* The tmp path was consumed */
+      ot_cleanup_unlinkat_clear (tmp_path);
     }
 
   return TRUE;
 }
 
+
 /* Given either a file or symlink, apply the final metadata to it depending on
  * the repository mode. Note that @checksum is assumed to have been validated by
  * the caller.
  */
 static gboolean
-commit_loose_content_object (OstreeRepo        *self,
+commit_loose_regfile_object (OstreeRepo        *self,
                              const char        *checksum,
-                             const char        *temp_filename,
+                             GLnxTmpfile       *tmpf,
-                             gboolean           object_is_symlink,
                              guint32            uid,
                              guint32            gid,
                              guint32            mode,
                              GVariant          *xattrs,
-                             int                fd,
                              GCancellable      *cancellable,
                              GError           **error)
 {
@@ -202,86 +223,53 @@ commit_loose_content_object (OstreeRepo        *self,
   if (self->mode == OSTREE_REPO_MODE_ARCHIVE_Z2
       && self->target_owner_uid != -1)
     {
-      if (fd != -1)
+      if (fchown (tmpf->fd, self->target_owner_uid, self->target_owner_gid) < 0)
-        {
-          if (fchown (fd, self->target_owner_uid, self->target_owner_gid) < 0)
         return glnx_throw_errno_prefix (error, "fchown");
     }
-      else if (G_UNLIKELY (fchownat (self->tmp_dir_fd, temp_filename,
+  else if (self->mode == OSTREE_REPO_MODE_BARE)
-                                     self->target_owner_uid,
-                                     self->target_owner_gid,
-                                     AT_SYMLINK_NOFOLLOW) == -1))
-        return glnx_throw_errno_prefix (error, "fchownat");
-    }
-
-  /* Special handling for symlinks in bare repositories */
-  if (object_is_symlink && self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
     {
-      /* We don't store the metadata in bare-user-only, so we're done. */
+      if (TEMP_FAILURE_RETRY (fchown (tmpf->fd, uid, gid)) < 0)
-    }
-  else if (object_is_symlink && self->mode == OSTREE_REPO_MODE_BARE)
-    {
-      /* Now that we know the checksum is valid, apply uid/gid, mode bits,
-       * and extended attributes.
-       *
-       * Note, this does not apply for bare-user repos, as they store symlinks
-       * as regular files.
-       */
-      if (G_UNLIKELY (fchownat (self->tmp_dir_fd, temp_filename,
-                                uid, gid,
-                                AT_SYMLINK_NOFOLLOW) == -1))
-        return glnx_throw_errno_prefix (error, "fchownat");
-
-      if (xattrs != NULL)
-        {
-          ot_security_smack_reset_dfd_name (self->tmp_dir_fd, temp_filename);
-          if (!glnx_dfd_name_set_all_xattrs (self->tmp_dir_fd, temp_filename,
-                                               xattrs, cancellable, error))
-            return FALSE;
-        }
-    }
-  else
-    {
-      if (self->mode == OSTREE_REPO_MODE_BARE)
-        {
-          if (TEMP_FAILURE_RETRY (fchown (fd, uid, gid)) < 0)
         return glnx_throw_errno_prefix (error, "fchown");
 
-          if (TEMP_FAILURE_RETRY (fchmod (fd, mode)) < 0)
+      if (TEMP_FAILURE_RETRY (fchmod (tmpf->fd, mode)) < 0)
         return glnx_throw_errno_prefix (error, "fchmod");
 
       if (xattrs)
         {
-              ot_security_smack_reset_fd (fd);
+          ot_security_smack_reset_fd (tmpf->fd);
-              if (!glnx_fd_set_all_xattrs (fd, xattrs, cancellable, error))
+          if (!glnx_fd_set_all_xattrs (tmpf->fd, xattrs, cancellable, error))
             return FALSE;
         }
     }
   else if (self->mode == OSTREE_REPO_MODE_BARE_USER)
     {
-          if (!write_file_metadata_to_xattr (fd, uid, gid, mode, xattrs, error))
+      if (!write_file_metadata_to_xattr (tmpf->fd, uid, gid, mode, xattrs, error))
         return FALSE;
 
-          if (!object_is_symlink)
-            {
       /* Note that previously this path added `| 0755` which made every
        * file executable, see
        * https://github.com/ostreedev/ostree/issues/907
+       *
+       * Again here, symlinks in bare-user are a hairy special case; only do a
+       * chmod for a *real* regular file, otherwise we'll take the default 0644.
        */
+      if (S_ISREG (mode))
+        {
           const mode_t content_mode = (mode & (S_IFREG | 0775));
-              if (fchmod (fd, content_mode) < 0)
+          if (fchmod (tmpf->fd, content_mode) < 0)
             return glnx_throw_errno_prefix (error, "fchmod");
         }
+      else
+        g_assert (S_ISLNK (mode));
     }
-      else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY
+  else if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
-               && !object_is_symlink)
     {
       guint32 invalid_modebits = (mode & ~S_IFMT) & ~0775;
       if (invalid_modebits > 0)
         return glnx_throw (error, "Invalid mode 0%04o with bits 0%04o in bare-user-only repository",
                            mode, invalid_modebits);
 
-          if (fchmod (fd, mode) < 0)
+      if (fchmod (tmpf->fd, mode) < 0)
         return glnx_throw_errno_prefix (error, "fchmod");
     }
 
@@ -292,7 +280,7 @@ commit_loose_content_object (OstreeRepo        *self,
        * set the modification time to OSTREE_TIMESTAMP.
        */
       const struct timespec times[2] = { { OSTREE_TIMESTAMP, UTIME_OMIT }, { OSTREE_TIMESTAMP, 0} };
-          if (TEMP_FAILURE_RETRY (futimens (fd, times)) < 0)
+      if (TEMP_FAILURE_RETRY (futimens (tmpf->fd, times)) < 0)
         return glnx_throw_errno_prefix (error, "futimens");
     }
 
@@ -301,14 +289,12 @@ commit_loose_content_object (OstreeRepo        *self,
    */
   if (!self->in_transaction && !self->disable_fsync)
     {
-          if (fsync (fd) == -1)
+      if (fsync (tmpf->fd) == -1)
         return glnx_throw_errno_prefix (error, "fsync");
     }
-    }
 
-  if (!_ostree_repo_commit_loose_final (self, checksum, OSTREE_OBJECT_TYPE_FILE,
+  if (!_ostree_repo_commit_tmpf_final (self, checksum, OSTREE_OBJECT_TYPE_FILE,
-                                        self->tmp_dir_fd, fd, temp_filename,
+                                       tmpf, cancellable, error))
-                                        cancellable, error))
     return FALSE;
 
   return TRUE;
@@ -453,7 +439,7 @@ gboolean
 _ostree_repo_open_content_bare (OstreeRepo          *self,
                                 const char          *checksum,
                                 guint64              content_len,
-                                OtTmpfile           *out_tmpf,
+                                GLnxTmpfile         *out_tmpf,
                                 gboolean            *out_have_object,
                                 GCancellable        *cancellable,
                                 GError             **error)
@@ -471,14 +457,14 @@ _ostree_repo_open_content_bare (OstreeRepo          *self,
       return TRUE;
     }
 
-  return ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
+  return glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
                                         out_tmpf, error);
 }
 
 gboolean
 _ostree_repo_commit_trusted_content_bare (OstreeRepo          *self,
                                           const char          *checksum,
-                                          OtTmpfile           *tmpf,
+                                          GLnxTmpfile         *tmpf,
                                           guint32              uid,
                                           guint32              gid,
                                           guint32              mode,
@@ -492,57 +478,45 @@ _ostree_repo_commit_trusted_content_bare (OstreeRepo          *self,
   if (!tmpf->initialized || tmpf->fd == -1)
     return TRUE;
 
-  if (!commit_loose_content_object (self, checksum,
+  return commit_loose_regfile_object (self, checksum,
-                                    tmpf->path,
+                                      tmpf, uid, gid, mode, xattrs,
-                                    FALSE, uid, gid, mode,
+                                      cancellable, error);
-                                    xattrs, tmpf->fd,
-                                    cancellable, error))
-    return glnx_prefix_error (error, "Writing object %s.%s", checksum, ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE));
-  /* The path was consumed */
-  g_clear_pointer (&tmpf->path, g_free);
-  tmpf->initialized = FALSE;
-  return TRUE;
 }
 
 static gboolean
 create_regular_tmpfile_linkable_with_content (OstreeRepo *self,
                                               guint64 length,
                                               GInputStream *input,
-                                              int *out_fd,
+                                              GLnxTmpfile *out_tmpf,
-                                              char **out_path,
                                               GCancellable *cancellable,
                                               GError **error)
 {
-  glnx_fd_close int temp_fd = -1;
+  g_auto(GLnxTmpfile) tmpf = { 0, };
-  g_autofree char *temp_filename = NULL;
-
   if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
-                                      &temp_fd, &temp_filename,
+                                      &tmpf, error))
-                                      error))
     return FALSE;
 
-  if (!ot_fallocate (temp_fd, length, error))
+  if (!ot_fallocate (tmpf.fd, length, error))
     return FALSE;
 
   if (G_IS_FILE_DESCRIPTOR_BASED (input))
     {
       int infd = g_file_descriptor_based_get_fd ((GFileDescriptorBased*) input);
-      if (glnx_regfile_copy_bytes (infd, temp_fd, (off_t)length, TRUE) < 0)
+      if (glnx_regfile_copy_bytes (infd, tmpf.fd, (off_t)length, TRUE) < 0)
         return glnx_throw_errno_prefix (error, "regfile copy");
     }
   else
     {
-      g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (temp_fd, FALSE);
+      g_autoptr(GOutputStream) temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);
       if (g_output_stream_splice (temp_out, input, 0,
                                   cancellable, error) < 0)
         return FALSE;
     }
 
-  if (fchmod (temp_fd, 0644) < 0)
+  if (fchmod (tmpf.fd, 0644) < 0)
     return glnx_throw_errno_prefix (error, "fchmod");
 
-  *out_fd = temp_fd; temp_fd = -1;
+  *out_tmpf = tmpf; tmpf.initialized = FALSE;
-  *out_path = g_steal_pointer (&temp_filename);
   return TRUE;
 }
 
@@ -615,47 +589,46 @@ write_content_object (OstreeRepo         *self,
    * could potentially cause the system to make a temporary setuid
    * binary with trailing garbage, creating a window on the local
    * system where a malicious setuid binary exists.
-   */
+   *
-  /* These variables are almost equivalent to OtTmpfile, except
+   * We use GLnxTmpfile for regular files, and OtCleanupUnlinkat for symlinks.
-   * temp_filename might also be a symlink.  Hence the CleanupUnlinkat
-   * which handles that case.
    */
   g_auto(OtCleanupUnlinkat) tmp_unlinker = { self->tmp_dir_fd, NULL };
-  glnx_fd_close int temp_fd = -1;
+  g_auto(GLnxTmpfile) tmpf = { 0, };
   gssize unpacked_size = 0;
   gboolean indexable = FALSE;
-  if ((_ostree_repo_mode_is_bare (repo_mode)) && !phys_object_is_symlink)
+  /* Is it a symlink physically? */
+  if (phys_object_is_symlink)
     {
-      if (!create_regular_tmpfile_linkable_with_content (self, size, file_input,
+      /* This will not be hit for bare-user or archive */
-                                                         &temp_fd, &tmp_unlinker.path,
+      g_assert (self->mode == OSTREE_REPO_MODE_BARE || self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY);
-                                                         cancellable, error))
-        return FALSE;
-    }
-  else if (_ostree_repo_mode_is_bare (repo_mode) && phys_object_is_symlink)
-    {
-      /* Note: This will not be hit for bare-user mode because its converted to a
-         regular file and take the branch above */
       if (!_ostree_make_temporary_symlink_at (self->tmp_dir_fd,
                                               g_file_info_get_symlink_target (file_info),
                                               &tmp_unlinker.path,
                                               cancellable, error))
         return FALSE;
     }
-  else if (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2)
+  else if (repo_mode != OSTREE_REPO_MODE_ARCHIVE_Z2)
+    {
+      if (!create_regular_tmpfile_linkable_with_content (self, size, file_input,
+                                                         &tmpf, cancellable, error))
+        return FALSE;
+    }
+  else
     {
       g_autoptr(GVariant) file_meta = NULL;
       g_autoptr(GConverter) zlib_compressor = NULL;
       g_autoptr(GOutputStream) compressed_out_stream = NULL;
       g_autoptr(GOutputStream) temp_out = NULL;
 
+      g_assert (repo_mode == OSTREE_REPO_MODE_ARCHIVE_Z2);
+
       if (self->generate_sizes)
         indexable = TRUE;
 
       if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
-                                          &temp_fd, &tmp_unlinker.path,
+                                          &tmpf, error))
-                                          error))
         return FALSE;
-      temp_out = g_unix_output_stream_new (temp_fd, FALSE);
+      temp_out = g_unix_output_stream_new (tmpf.fd, FALSE);
 
       file_meta = _ostree_zlib_file_header_new (file_info, xattrs);
 
@@ -679,7 +652,7 @@ write_content_object (OstreeRepo         *self,
       if (!g_output_stream_flush (temp_out, cancellable, error))
         return FALSE;
 
-      if (fchmod (temp_fd, 0644) < 0)
+      if (fchmod (tmpf.fd, 0644) < 0)
         return glnx_throw_errno_prefix (error, "fchmod");
     }
 
@@ -720,23 +693,61 @@ write_content_object (OstreeRepo         *self,
   const guint32 uid = g_file_info_get_attribute_uint32 (file_info, "unix::uid");
   const guint32 gid = g_file_info_get_attribute_uint32 (file_info, "unix::gid");
   const guint32 mode = g_file_info_get_attribute_uint32 (file_info, "unix::mode");
-  if (!commit_loose_content_object (self, actual_checksum,
+  /* Is it "physically" a symlink? */
-                                    tmp_unlinker.path,
+  if (phys_object_is_symlink)
-                                    object_file_type == G_FILE_TYPE_SYMBOLIC_LINK,
+    {
+      if (self->mode == OSTREE_REPO_MODE_BARE_USER_ONLY)
+        {
+          /* We don't store the metadata in bare-user-only, so we're done. */
+        }
+      else if (self->mode == OSTREE_REPO_MODE_BARE)
+        {
+          /* Now that we know the checksum is valid, apply uid/gid, mode bits,
+           * and extended attributes.
+           *
+           * Note, this does not apply for bare-user repos, as they store symlinks
+           * as regular files.
+           */
+          if (G_UNLIKELY (fchownat (self->tmp_dir_fd, tmp_unlinker.path,
+                                    uid, gid, AT_SYMLINK_NOFOLLOW) == -1))
+            return glnx_throw_errno_prefix (error, "fchownat");
+
+          if (xattrs != NULL)
+            {
+              ot_security_smack_reset_dfd_name (self->tmp_dir_fd, tmp_unlinker.path);
+              if (!glnx_dfd_name_set_all_xattrs (self->tmp_dir_fd, tmp_unlinker.path,
+                                                 xattrs, cancellable, error))
+                return FALSE;
+            }
+        }
+      else
+        {
+          /* We don't do symlinks in archive or bare-user */
+          g_assert_not_reached ();
+        }
+
+      if (!_ostree_repo_commit_path_final (self, actual_checksum, OSTREE_OBJECT_TYPE_FILE,
+                                           &tmp_unlinker,
+                                           cancellable, error))
+        return FALSE;
+    }
+  else
+    {
+      /* This path is for regular files */
+      if (!commit_loose_regfile_object (self, actual_checksum, &tmpf,
                                         uid, gid, mode,
-                                    xattrs, temp_fd,
+                                        xattrs,
                                         cancellable, error))
         return glnx_prefix_error (error, "Writing object %s.%s", actual_checksum,
                                   ostree_object_type_to_string (OSTREE_OBJECT_TYPE_FILE));
-  /* Clear the unlinker, it was consumed */
+    }
-  ot_cleanup_unlinkat_clear (&tmp_unlinker);
 
   /* Update size metadata if configured */
   if (indexable && object_file_type == G_FILE_TYPE_REGULAR)
     {
       struct stat stbuf;
 
-      if (!glnx_fstat (temp_fd, &stbuf, error))
+      if (!glnx_fstat (tmpf.fd, &stbuf, error))
         return FALSE;
 
       repo_store_size_entry (self, actual_checksum, unpacked_size, stbuf.st_size);
@@ -832,8 +843,8 @@ write_metadata_object (OstreeRepo         *self,
     }
 
   /* Write the metadata to a temporary file */
-  g_auto(OtTmpfile) tmpf = { 0, };
+  g_auto(GLnxTmpfile) tmpf = { 0, };
-  if (!ot_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
+  if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_WRONLY|O_CLOEXEC,
                                       &tmpf, error))
     return FALSE;
   if (!ot_fallocate (tmpf.fd, len, error))
@@ -844,12 +855,9 @@ write_metadata_object (OstreeRepo         *self,
     return glnx_throw_errno_prefix (error, "fchmod");
 
   /* And commit it into place */
-  if (!_ostree_repo_commit_loose_final (self, actual_checksum, objtype,
+  if (!_ostree_repo_commit_tmpf_final (self, actual_checksum, objtype,
-                                        self->tmp_dir_fd, tmpf.fd, tmpf.path,
+                                       &tmpf, cancellable, error))
-                                        cancellable, error))
     return FALSE;
-  /* The temp path was consumed */
-  g_clear_pointer (&tmpf.path, g_free);
 
   if (objtype == OSTREE_OBJECT_TYPE_COMMIT)
     {

src/libostree/ostree-repo-private.h

@@ -296,12 +296,18 @@ _ostree_repo_verify_commit_internal (OstreeRepo    *self,
                                      GError       **error);
 
 gboolean
-_ostree_repo_commit_loose_final (OstreeRepo        *self,
+_ostree_repo_commit_tmpf_final (OstreeRepo        *self,
                                 const char        *checksum,
                                 OstreeObjectType   objtype,
-                                 int                temp_dfd,
+                                GLnxTmpfile       *tmpf,
-                                 int                fd,
+                                GCancellable      *cancellable,
-                                 const char        *temp_filename,
+                                GError           **error);
+
+gboolean
+_ostree_repo_commit_path_final (OstreeRepo        *self,
+                                const char        *checksum,
+                                OstreeObjectType   objtype,
+                                OtCleanupUnlinkat *tmp_path,
                                 GCancellable      *cancellable,
                                 GError           **error);
 
@@ -309,7 +315,7 @@ gboolean
 _ostree_repo_open_content_bare (OstreeRepo          *self,
                                 const char          *checksum,
                                 guint64              content_len,
-                                OtTmpfile           *out_tmpf,
+                                GLnxTmpfile         *out_tmpf,
                                 gboolean            *out_have_object,
                                 GCancellable        *cancellable,
                                 GError             **error);
@@ -317,7 +323,7 @@ _ostree_repo_open_content_bare (OstreeRepo          *self,
 gboolean
 _ostree_repo_commit_trusted_content_bare (OstreeRepo          *self,
                                           const char          *checksum,
-                                          OtTmpfile           *tmpf,
+                                          GLnxTmpfile         *tmpf,
                                           guint32              uid,
                                           guint32              gid,
                                           guint32              mode,

src/libostree/ostree-repo-pull.c

@@ -941,12 +941,10 @@ content_fetch_on_complete (GObject        *object,
 
       if (!have_object)
         {
-          if (!_ostree_repo_commit_loose_final (pull_data->repo, checksum, OSTREE_OBJECT_TYPE_FILE,
+          if (!_ostree_repo_commit_path_final (pull_data->repo, checksum, objtype,
-                                                tmp_unlinker.dfd, -1, tmp_unlinker.path,
+                                               &tmp_unlinker,
                                                cancellable, error))
             goto out;
-          /* The path was consumed */
-          ot_cleanup_unlinkat_clear (&tmp_unlinker);
         }
       pull_data->n_fetched_content++;
     }
@@ -954,6 +952,7 @@ content_fetch_on_complete (GObject        *object,
     {
       /* Non-mirroring path */
 
+      /* If it appears corrupted, we'll delete it below */
       if (!ostree_content_file_parse_at (TRUE, _ostree_fetcher_get_dfd (fetcher),
                                          tmp_unlinker.path, FALSE,
                                          &file_in, &file_info, &xattrs,

src/libostree/ostree-repo-static-delta-compilation.c

@@ -437,29 +437,25 @@ get_unpacked_unlinked_content (OstreeRepo       *repo,
                                GCancellable     *cancellable,
                                GError          **error)
 {
-  g_autofree char *tmpname = NULL;
+  g_auto(GLnxTmpfile) tmpf = { 0, };
-  glnx_fd_close int fd = -1;
   g_autoptr(GBytes) ret_content = NULL;
   g_autoptr(GInputStream) istream = NULL;
   g_autoptr(GOutputStream) out = NULL;
 
   if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, "/tmp", O_RDWR | O_CLOEXEC,
-                                      &fd, &tmpname, error))
+                                      &tmpf, error))
     return FALSE;
-  /* We don't need the file name */
-  if (tmpname)
-    (void) unlinkat (AT_FDCWD, tmpname, 0);
 
   if (!ostree_repo_load_file (repo, checksum, &istream, NULL, NULL,
                               cancellable, error))
     return FALSE;
 
-  out = g_unix_output_stream_new (fd, FALSE);
+  out = g_unix_output_stream_new (tmpf.fd, FALSE);
   if (g_output_stream_splice (out, istream, G_OUTPUT_STREAM_SPLICE_CLOSE_TARGET,
                               cancellable, error) < 0)
     return FALSE;
 
-  { g_autoptr(GMappedFile) mfile = g_mapped_file_new_from_fd (fd, FALSE, error);
+  { g_autoptr(GMappedFile) mfile = g_mapped_file_new_from_fd (tmpf.fd, FALSE, error);
     if (!mfile)
       return FALSE;
     ret_content = g_mapped_file_get_bytes (mfile);
@@ -1163,6 +1159,15 @@ get_fallback_headers (OstreeRepo               *self,
   return TRUE;
 }
 
+static inline void
+glnx_tmpfile_clear_p (void *ptr)
+{
+  GLnxTmpfile *tmpf = ptr;
+  if (!tmpf)
+    return;
+  glnx_tmpfile_clear (tmpf);
+}
+
 /**
  * ostree_repo_static_delta_generate:
  * @self: Repo
@@ -1213,7 +1218,6 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
   guint64 total_compressed_size = 0;
   guint64 total_uncompressed_size = 0;
   g_autoptr(GVariantBuilder) part_headers = NULL;
-  g_autoptr(GArray) part_temp_fds = NULL;
   g_autoptr(GPtrArray) part_temp_paths = NULL;
   g_autoptr(GVariant) delta_descriptor = NULL;
   g_autoptr(GVariant) to_commit = NULL;
@@ -1325,8 +1329,7 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
     }
 
   part_headers = g_variant_builder_new (G_VARIANT_TYPE ("a" OSTREE_STATIC_DELTA_META_ENTRY_FORMAT));
-  part_temp_paths = g_ptr_array_new_with_free_func (g_free);
+  part_temp_paths = g_ptr_array_new_with_free_func (glnx_tmpfile_clear_p);
-  part_temp_fds = g_array_new (FALSE, TRUE, sizeof(int));
   for (i = 0; i < builder.parts->len; i++)
     {
       OstreeStaticDeltaPartBuilder *part_builder = builder.parts->pdata[i];
@@ -1399,17 +1402,15 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
         }
       else
         {
-          char *part_tempfile;
+          GLnxTmpfile *part_tmpf = g_new0 (GLnxTmpfile, 1);
-          int part_temp_fd;
 
           if (!glnx_open_tmpfile_linkable_at (tmp_dfd, ".", O_WRONLY | O_CLOEXEC,
-                                              &part_temp_fd, &part_tempfile, error))
+                                              part_tmpf, error))
             goto out;
 
-          /* Transfer tempfile ownership to arrays */
+          /* Transfer tempfile ownership */
-          g_array_append_val (part_temp_fds, part_temp_fd);
+          part_temp_outstream = g_unix_output_stream_new (part_tmpf->fd, FALSE);
-          g_ptr_array_add (part_temp_paths, g_steal_pointer (&part_tempfile));
+          g_ptr_array_add (part_temp_paths, g_steal_pointer (&part_tmpf));
-          part_temp_outstream = g_unix_output_stream_new (part_temp_fd, FALSE);
         }
 
       part_in = ot_variant_read (delta_part);
@@ -1468,17 +1469,16 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
     {
       g_autofree char *partstr = g_strdup_printf ("%u", i);
       /* Take ownership of the path/fd here */
-      g_autofree char *path = g_steal_pointer (&part_temp_paths->pdata[i]);
+      g_auto(GLnxTmpfile) tmpf = *((GLnxTmpfile*)part_temp_paths->pdata[i]);
-      glnx_fd_close int fd = g_array_index (part_temp_fds, int, i);
+      g_clear_pointer (&(part_temp_paths->pdata[i]), g_free);
-      g_array_index (part_temp_fds, int, i) = -1;
 
-      if (fchmod (fd, 0644) < 0)
+      if (fchmod (tmpf.fd, 0644) < 0)
         {
           glnx_set_error_from_errno (error);
           goto out;
         }
 
-      if (!glnx_link_tmpfile_at (tmp_dfd, GLNX_LINK_TMPFILE_REPLACE, fd, path,
+      if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE,
                                  descriptor_dfd, partstr, error))
         goto out;
     }
@@ -1538,14 +1538,6 @@ ostree_repo_static_delta_generate (OstreeRepo                   *self,
 
   ret = TRUE;
  out:
-  if (part_temp_fds)
-    for (i = 0; i < part_temp_fds->len; i++)
-      {
-        int fd = g_array_index (part_temp_fds, int, i);
-        if (fd == -1)
-          continue;
-        (void) close (fd);
-      }
   g_clear_pointer (&builder.parts, g_ptr_array_unref);
   g_clear_pointer (&builder.fallback_objects, g_ptr_array_unref);
   return ret;

src/libostree/ostree-repo-static-delta-processing.c

@@ -56,7 +56,7 @@ typedef struct {
   GError        **async_error;
 
   OstreeObjectType output_objtype;
-  OtTmpfile        tmpf;
+  GLnxTmpfile      tmpf;
   guint64          content_size;
   GOutputStream   *content_out;
   GChecksum       *content_checksum;
@@ -281,7 +281,7 @@ _ostree_static_delta_part_execute (OstreeRepo      *repo,
 
   ret = TRUE;
  out:
-  ot_tmpfile_clear (&state->tmpf);
+  glnx_tmpfile_clear (&state->tmpf);
   g_clear_object (&state->content_out);
   g_clear_pointer (&state->content_checksum, g_checksum_free);
   return ret;

src/libostree/ostree-repo.c

@@ -3862,8 +3862,7 @@ sign_data (OstreeRepo     *self,
            GError        **error)
 {
   gboolean ret = FALSE;
-  glnx_fd_close int tmp_fd = -1;
+  g_auto(GLnxTmpfile) tmpf = { 0, };
-  g_autofree char *tmp_path = NULL;
   g_autoptr(GOutputStream) tmp_signature_output = NULL;
   gpgme_ctx_t context = NULL;
   g_autoptr(GBytes) ret_signature = NULL;
@@ -3874,9 +3873,9 @@ sign_data (OstreeRepo     *self,
   g_autoptr(GMappedFile) signature_file = NULL;
   
   if (!glnx_open_tmpfile_linkable_at (self->tmp_dir_fd, ".", O_RDWR | O_CLOEXEC,
-                                      &tmp_fd, &tmp_path, error))
+                                      &tmpf, error))
     goto out;
-  tmp_signature_output = g_unix_output_stream_new (tmp_fd, FALSE);
+  tmp_signature_output = g_unix_output_stream_new (tmpf.fd, FALSE);
 
   context = ot_gpgme_new_ctx (homedir, error);
   if (!context)
@@ -3930,7 +3929,7 @@ sign_data (OstreeRepo     *self,
   if (!g_output_stream_close (tmp_signature_output, cancellable, error))
     goto out;
   
-  signature_file = g_mapped_file_new_from_fd (tmp_fd, FALSE, error);
+  signature_file = g_mapped_file_new_from_fd (tmpf.fd, FALSE, error);
   if (!signature_file)
     goto out;
   ret_signature = g_mapped_file_get_bytes (signature_file);

src/libotutil/ot-fs-utils.c

@@ -25,61 +25,6 @@
 #include <sys/xattr.h>
 #include <gio/gunixinputstream.h>
 
-/* Before https://github.com/GNOME/libglnx/commit/9929adc, the libglnx
- * tmpfile API made it hard to clean up tmpfiles in failure cases.
- * it's API breaking. Carry the fix here until we're ready to fully port.
- */
-void
-ot_tmpfile_clear (OtTmpfile *tmpf)
-{
-  if (!tmpf->initialized)
-    return;
-  if (tmpf->fd == -1)
-    return;
-  (void) close (tmpf->fd);
-  /* If ->path is set, we're likely aborting due to an error. Clean it up */
-  if (tmpf->path)
-    {
-      (void) unlinkat (tmpf->src_dfd, tmpf->path, 0);
-      g_free (tmpf->path);
-    }
-}
-
-gboolean
-ot_open_tmpfile_linkable_at (int dfd,
-                             const char *subpath,
-                             int flags,
-                             OtTmpfile *out_tmpf,
-                             GError **error)
-{
-  if (!glnx_open_tmpfile_linkable_at (dfd, subpath, flags, &out_tmpf->fd, &out_tmpf->path, error))
-    return FALSE;
-  out_tmpf->initialized = TRUE;
-  out_tmpf->src_dfd = dfd;
-  return TRUE;
-}
-
-gboolean
-ot_link_tmpfile_at (OtTmpfile *tmpf,
-                    GLnxLinkTmpfileReplaceMode mode,
-                    int target_dfd,
-                    const char *target,
-                    GError **error)
-{
-  g_return_val_if_fail (tmpf->initialized, FALSE);
-  glnx_fd_close int fd = glnx_steal_fd (&tmpf->fd);
-  if (!glnx_link_tmpfile_at (tmpf->src_dfd, mode, fd, tmpf->path,
-                             target_dfd, target, error))
-    {
-      if (tmpf->path)
-        (void) unlinkat (tmpf->src_dfd, tmpf->path, 0);
-      tmpf->initialized = FALSE;
-      return FALSE;
-    }
-  tmpf->initialized = FALSE;
-  return TRUE;
-}
-
 /* Convert a fd-relative path to a GFile* - use
  * for legacy code.
  */
@@ -213,9 +158,8 @@ ot_dfd_iter_init_allow_noent (int dfd,
       *out_exists = FALSE;
       return TRUE;
     }
-  if (!glnx_dirfd_iterator_init_take_fd (fd, dfd_iter, error))
+  if (!glnx_dirfd_iterator_init_take_fd (&fd, dfd_iter, error))
     return FALSE;
-  fd = -1;
   *out_exists = TRUE;
   return TRUE;
 }

src/libotutil/ot-fs-utils.h

@@ -25,31 +25,6 @@
 
 G_BEGIN_DECLS
 
-/* This is a copy of https://github.com/GNOME/libglnx/pull/46 until we
- * can do a full port; see https://github.com/ostreedev/ostree/pull/861 */
-typedef struct {
-  gboolean initialized;
-  int src_dfd;
-  int fd;
-  char *path;
-} OtTmpfile;
-void ot_tmpfile_clear (OtTmpfile *tmpf);
-G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtTmpfile, ot_tmpfile_clear);
-
-gboolean
-ot_open_tmpfile_linkable_at (int dfd,
-                             const char *subpath,
-                             int flags,
-                             OtTmpfile *out_tmpf,
-                             GError **error);
-gboolean
-ot_link_tmpfile_at (OtTmpfile *tmpf,
-                    GLnxLinkTmpfileReplaceMode flags,
-                    int target_dfd,
-                    const char *target,
-                    GError **error);
-
-
 /* A little helper to call unlinkat() as a cleanup
  * function.  Mostly only necessary to handle
  * deletion of temporary symlinks.
@@ -76,7 +51,6 @@ ot_cleanup_unlinkat (OtCleanupUnlinkat *cleanup)
 }
 G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(OtCleanupUnlinkat, ot_cleanup_unlinkat);
 
-
 GFile * ot_fdrel_to_gfile (int dfd, const char *path);
 
 gboolean ot_readlinkat_gfile_info (int             dfd,

src/ostree/ot-remote-cookie-util.c

@@ -202,7 +202,7 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
 {
   gboolean found = FALSE;
 #ifdef HAVE_LIBCURL
-  g_auto(OtTmpfile) tmpf = { 0, };
+  g_auto(GLnxTmpfile) tmpf = { 0, };
   g_autofree char *dnbuf = NULL;
   const char *dn = NULL;
   g_autoptr(OtCookieParser) parser = NULL;
@@ -212,7 +212,7 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
 
   dnbuf = g_strdup (jar_path);
   dn = dirname (dnbuf);
-  if (!ot_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC,
+  if (!glnx_open_tmpfile_linkable_at (AT_FDCWD, dn, O_WRONLY | O_CLOEXEC,
                                       &tmpf, error))
     return FALSE;
 
@@ -232,7 +232,7 @@ ot_delete_cookie_at (int dfd, const char *jar_path,
         return glnx_throw_errno_prefix (error, "write");
     }
 
-  if (!ot_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE,
+  if (!glnx_link_tmpfile_at (&tmpf, GLNX_LINK_TMPFILE_REPLACE,
                              AT_FDCWD, jar_path,
                              error))
     return FALSE;