From 544d79fd3f2d9f5dc7279b2c174cb9aab4ce7090 Mon Sep 17 00:00:00 2001
From: Jonathan Lebon <jonathan@jlebon.com>
Date: Wed, 5 Dec 2018 16:51:40 -0500
Subject: [PATCH] compose: Fix EBADF in unified core mode without cachedir

If no cache dir is given in the workdir, we would alias the cache dir fd
to the workdir fd. But of course, this meant that we'd try to close the
same fd twice when freeing the compose context. Instead, let's just copy
the fd as is also done in the non-unified path.

Closes: #1697

Closes: #1698
Approved by: lucab
---
 src/app/rpmostree-compose-builtin-tree.c  | 8 +++++---
 tests/compose-tests/test-basic-unified.sh | 5 +++++
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/src/app/rpmostree-compose-builtin-tree.c b/src/app/rpmostree-compose-builtin-tree.c
index 2ae031d7..354484d4 100644
--- a/src/app/rpmostree-compose-builtin-tree.c
+++ b/src/app/rpmostree-compose-builtin-tree.c
@@ -586,7 +586,9 @@ rpm_ostree_compose_context_new (const char    *treefile_pathstr,
                 return FALSE;
             }
 
-          self->cachedir_dfd = self->workdir_tmp.fd;
+          self->cachedir_dfd = fcntl (self->workdir_tmp.fd, F_DUPFD_CLOEXEC, 3);
+          if (self->cachedir_dfd < 0)
+            return glnx_throw_errno_prefix (error, "fcntl");
         }
 
       self->pkgcache_repo = ostree_repo_create_at (self->cachedir_dfd, "pkgcache-repo",
@@ -604,7 +606,7 @@ rpm_ostree_compose_context_new (const char    *treefile_pathstr,
       if (!self->build_repo)
         return glnx_prefix_error (error, "Creating repo-build");
 
-      /* Note special handling of this aliasing in _finalize() */
+      /* Note special handling of this aliasing in rpm_ostree_tree_compose_context_free() */
       self->workdir_dfd = self->workdir_tmp.fd;
     }
   else
@@ -613,7 +615,7 @@ rpm_ostree_compose_context_new (const char    *treefile_pathstr,
         {
           if (!glnx_mkdtempat (AT_FDCWD, "/var/tmp/rpm-ostree.XXXXXX", 0700, &self->workdir_tmp, error))
             return FALSE;
-          /* Note special handling of this aliasing in _finalize() */
+          /* Note special handling of this aliasing in rpm_ostree_tree_compose_context_free() */
           self->workdir_dfd = self->workdir_tmp.fd;
         }
       else
diff --git a/tests/compose-tests/test-basic-unified.sh b/tests/compose-tests/test-basic-unified.sh
index c5559ce8..81d43f19 100755
--- a/tests/compose-tests/test-basic-unified.sh
+++ b/tests/compose-tests/test-basic-unified.sh
@@ -26,6 +26,11 @@ cat > metadata.json <<EOF
 EOF
 runcompose --ex-unified-core --add-metadata-from-json metadata.json
 
+# Run it again, but without RPMOSTREE_PRESERVE_TMPDIR. Should be a no-op. This
+# exercises fd handling in the tree context.
+rpm-ostree compose tree ${compose_base_argv} ${treefile} "$@"
+echo "ok no cachedir"
+
 . ${dn}/libbasic-test.sh
 basic_test