diff --git a/glnx-dirfd.c b/glnx-dirfd.c index 29a3ff7c5fc811167d2b30aecbb5b3b5743dab11..b745e280992ea1f8e788ad1573adf02e158f546a 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 8e582fc1e0338ffb33a8acc96c0ff0a7e9fbc9a1..5e362fab27e297cbd654cdcbebd5ccb3c5205b57 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 37c982d7ba21a94f7ae8831464c362ac2659a987..1f94ce2ead182307a1a5556cc366e675132c9149 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); }