From 09577c63f7cc2dcb72bc1724d13c9c5052a78548 Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 12 Dec 2022 14:09:33 +0000 Subject: [PATCH] exports: Make _exports_path_expose produce a GError on failure This is a step towards allowing its direct and indirect callers to decide how serious the failure is, and debug or warn accordingly. Helps: https://github.com/flatpak/flatpak/issues/5205 Signed-off-by: Simon McVittie (cherry picked from commit 1b49de1890dfc92aacd9cb1d30beb1d87432d58a) --- common/flatpak-exports.c | 95 +++++++++++++++++++++++++++------------- 1 file changed, 64 insertions(+), 31 deletions(-) diff --git a/common/flatpak-exports.c b/common/flatpak-exports.c index b003180f..e328c20e 100644 --- a/common/flatpak-exports.c +++ b/common/flatpak-exports.c @@ -869,12 +869,18 @@ check_if_autofs_works (FlatpakExports *exports, return TRUE; } -/* We use level to avoid infinite recursion */ +/* We use level to avoid infinite recursion. + * + * Note that some of the errors produced by this function are "real errors" + * and should show up as a user-visible warning, but others are relatively + * uninteresting, and in general none are actually fatal: we prefer to + * continue with fewer paths exposed rather than failing to run. */ static gboolean _exports_path_expose (FlatpakExports *exports, int mode, const char *path, - int level) + int level, + GError **error) { g_autofree char *canonical = NULL; struct stat st; @@ -889,13 +895,15 @@ _exports_path_expose (FlatpakExports *exports, if (level > 40) /* 40 is the current kernel ELOOP check */ { - g_debug ("Expose too deep, bail"); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_TOO_MANY_LINKS, + "%s", g_strerror (ELOOP)); return FALSE; } if (!g_path_is_absolute (path)) { - g_debug ("Not exposing relative path %s", path); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_FILENAME, + _("An absolute path is required")); return FALSE; } @@ -904,34 +912,38 @@ _exports_path_expose (FlatpakExports *exports, if (o_path_fd == -1) { - g_debug ("Unable to open path %s to %s: %s", - path, export_mode_to_verb (mode), g_strerror (errno)); + int saved_errno = errno; + + /* Intentionally using G_IO_ERROR_NOT_FOUND even if errno is + * something different, so callers can suppress the warning in this + * relatively likely and uninteresting case: we don't particularly + * care whether this is happening as a result of ENOENT or EACCES + * or any other reason. */ + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + _("Unable to open path \"%s\": %s"), + path, g_strerror (saved_errno)); return FALSE; } if (fstat (o_path_fd, &st) != 0) - { - g_debug ("Unable to get file type of %s: %s", path, g_strerror (errno)); - return FALSE; - } + return glnx_throw (error, + _("Unable to get file type of \"%s\": %s"), + path, g_strerror (errno)); /* Don't expose weird things */ if (!(S_ISDIR (st.st_mode) || S_ISREG (st.st_mode) || S_ISLNK (st.st_mode) || S_ISSOCK (st.st_mode))) - { - g_debug ("%s has unsupported file type 0o%o", path, st.st_mode & S_IFMT); - return FALSE; - } + return glnx_throw (error, + _("File \"%s\" has unsupported type 0o%o"), + path, st.st_mode & S_IFMT); /* O_PATH + fstatfs is the magic that we need to statfs without automounting the target */ if (fstatfs (o_path_fd, &stfs) != 0) - { - g_debug ("Unable to get filesystem information for %s: %s", - path, g_strerror (errno)); - return FALSE; - } + return glnx_throw (error, + _("Unable to get filesystem information for \"%s\": %s"), + path, g_strerror (errno)); if (stfs.f_type == AUTOFS_SUPER_MAGIC || (G_UNLIKELY (exports->test_flags & FLATPAK_EXPORTS_TEST_FLAGS_AUTOFS) && @@ -939,7 +951,8 @@ _exports_path_expose (FlatpakExports *exports, { if (!check_if_autofs_works (exports, path)) { - g_debug ("ignoring blocking autofs path %s", path); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK, + _("Ignoring blocking autofs path \"%s\""), path); return FALSE; } } @@ -954,17 +967,22 @@ _exports_path_expose (FlatpakExports *exports, create the parents for them anyway */ if (flatpak_has_path_prefix (path, dont_export_in[i])) { - g_debug ("skipping export for path %s in unsupported prefix", path); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_MOUNTABLE_FILE, + _("Path \"%s\" is reserved by Flatpak"), + dont_export_in[i]); return FALSE; } } for (i = 0; flatpak_abs_usrmerged_dirs[i] != NULL; i++) { - /* Same as /usr, but for the directories that get merged into /usr */ + /* Same as /usr, but for the directories that get merged into /usr. + * Keep the translatable string here the same as the one above */ if (flatpak_has_path_prefix (path, flatpak_abs_usrmerged_dirs[i])) { - g_debug ("skipping export for path %s in a /usr-merged directory", path); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_MOUNTABLE_FILE, + _("Path \"%s\" is reserved by Flatpak"), + flatpak_abs_usrmerged_dirs[i]); return FALSE; } } @@ -988,8 +1006,8 @@ _exports_path_expose (FlatpakExports *exports, } else { - g_autoptr(GError) error = NULL; - g_autofree char *resolved = flatpak_exports_resolve_link_in_host (exports, path, &error); + g_autoptr(GError) local_error = NULL; + g_autofree char *resolved = flatpak_exports_resolve_link_in_host (exports, path, &local_error); g_autofree char *new_target = NULL; if (resolved) @@ -1003,7 +1021,7 @@ _exports_path_expose (FlatpakExports *exports, flatpak_debug2 ("Trying to export the target instead: %s", new_target); - if (_exports_path_expose (exports, mode, new_target, level + 1)) + if (_exports_path_expose (exports, mode, new_target, level + 1, &local_error)) { do_export_path (exports, path, FAKE_MODE_SYMLINK); return TRUE; @@ -1011,12 +1029,14 @@ _exports_path_expose (FlatpakExports *exports, flatpak_debug2 ("Could not export target %s, so ignoring %s", new_target, path); + g_propagate_error (error, g_steal_pointer (&local_error)); return FALSE; } else { - flatpak_debug2 ("%s is a symlink but we were unable to resolve it: %s", - path, error->message); + g_set_error (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND, + _("Unable to resolve symbolic link \"%s\": %s"), + path, local_error->message); return FALSE; } } @@ -1035,16 +1055,25 @@ flatpak_exports_add_path_expose (FlatpakExports *exports, FlatpakFilesystemMode mode, const char *path) { + g_autoptr(GError) local_error = NULL; + g_return_if_fail (mode > FLATPAK_FILESYSTEM_MODE_NONE); g_return_if_fail (mode <= FLATPAK_FILESYSTEM_MODE_LAST); - _exports_path_expose (exports, mode, path, 0); + + if (!_exports_path_expose (exports, mode, path, 0, &local_error)) + g_debug ("Unable to %s: \"%s\": %s", + export_mode_to_verb (mode), path, local_error->message); } void flatpak_exports_add_path_tmpfs (FlatpakExports *exports, const char *path) { - _exports_path_expose (exports, FAKE_MODE_TMPFS, path, 0); + g_autoptr(GError) local_error = NULL; + + if (!_exports_path_expose (exports, FAKE_MODE_TMPFS, path, 0, &local_error)) + g_debug ("Unable to %s: \"%s\": %s", + export_mode_to_verb (FAKE_MODE_TMPFS), path, local_error->message); } void @@ -1065,7 +1094,11 @@ void flatpak_exports_add_path_dir (FlatpakExports *exports, const char *path) { - _exports_path_expose (exports, FAKE_MODE_DIR, path, 0); + g_autoptr(GError) local_error = NULL; + + if (!_exports_path_expose (exports, FAKE_MODE_DIR, path, 0, &local_error)) + g_debug ("Unable to %s: \"%s\": %s", + export_mode_to_verb (FAKE_MODE_DIR), path, local_error->message); } void