Compare commits
No commits in common. 'i8c' and 'c9' have entirely different histories.
@ -1 +1 @@
|
|||||||
SOURCES/libosinfo-1.9.0.tar.xz
|
SOURCES/libosinfo-1.10.0.tar.xz
|
||||||
|
|||||||
@ -1 +1 @@
|
|||||||
1f928e1412ac29e044e6b02254c9d2b2bf969c31 SOURCES/libosinfo-1.9.0.tar.xz
|
b57c594b7f45a33575070df86862df78be2c4efb SOURCES/libosinfo-1.10.0.tar.xz
|
||||||
|
|||||||
@ -1,45 +0,0 @@
|
|||||||
From d3b1587f7b77b630bae8ab3f4760eda69bd7fe66 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Victor Toso <victortoso@redhat.com>
|
|
||||||
Date: Fri, 26 Nov 2021 17:36:09 +0100
|
|
||||||
Subject: [PATCH] loader: add check for unknown file type
|
|
||||||
|
|
||||||
So we can provide a meaningful error message in case the provided path
|
|
||||||
is not accessible to running process.
|
|
||||||
|
|
||||||
e.g: running HOME=/root osinfo-query os we would get
|
|
||||||
`Error loading OS data: Unexpected file type`
|
|
||||||
and now we get
|
|
||||||
`Error loading OS data: Can't read path /root/.config/osinfo`
|
|
||||||
|
|
||||||
This error was first hit with v2v [0] that was leaking $USER and $HOME
|
|
||||||
of root user when osinfo-query as vsdm user with `sudo -c vdsm`. The
|
|
||||||
example above is a simple way to show lack of permision of
|
|
||||||
osinfo-query to read the root's $HOME.
|
|
||||||
|
|
||||||
[0] https://bugzilla.redhat.com/show_bug.cgi?id=1901423
|
|
||||||
|
|
||||||
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1902720
|
|
||||||
Signed-off-by: Victor Toso <victortoso@redhat.com>
|
|
||||||
---
|
|
||||||
osinfo/osinfo_loader.c | 5 +++++
|
|
||||||
1 file changed, 5 insertions(+)
|
|
||||||
|
|
||||||
diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
|
|
||||||
index 3f04fa5..96ca6ee 100644
|
|
||||||
--- a/osinfo/osinfo_loader.c
|
|
||||||
+++ b/osinfo/osinfo_loader.c
|
|
||||||
@@ -2374,6 +2374,11 @@ static void osinfo_loader_find_files(OsinfoLoader *loader,
|
|
||||||
}
|
|
||||||
g_object_unref(ents);
|
|
||||||
g_list_free(children);
|
|
||||||
+ } else if (type == G_FILE_TYPE_UNKNOWN) {
|
|
||||||
+ g_autofree gchar *path = g_file_get_path(file);
|
|
||||||
+ g_autofree gchar *msg = g_strdup_printf("Can't read path %s", path);
|
|
||||||
+ OSINFO_LOADER_SET_ERROR(&error, msg);
|
|
||||||
+ g_propagate_error(err, error);
|
|
||||||
} else {
|
|
||||||
OSINFO_LOADER_SET_ERROR(&error, "Unexpected file type");
|
|
||||||
g_propagate_error(err, error);
|
|
||||||
--
|
|
||||||
2.33.1
|
|
||||||
|
|
||||||
@ -1,220 +0,0 @@
|
|||||||
From e5bdc6759195dbcfc4e7dcb02bf59190a3debe06 Mon Sep 17 00:00:00 2001
|
|
||||||
From: Laszlo Ersek <lersek@redhat.com>
|
|
||||||
Date: Wed, 9 Feb 2022 13:14:54 +0100
|
|
||||||
Subject: [PATCH] loader: work around lstat()/EACCES regression in
|
|
||||||
_g_local_file_info_get()
|
|
||||||
|
|
||||||
In glib commit 71e7b5800a31 ("Handle MLS selinux policy better",
|
|
||||||
2010-07-08), which was made for
|
|
||||||
<https://bugzilla.gnome.org/show_bug.cgi?id=623692>, an lstat() failure
|
|
||||||
with error code EACCES was *masked* in function _g_local_file_info_get().
|
|
||||||
|
|
||||||
Consequently, if osinfo_loader_find_files() calls g_file_query_info() on a
|
|
||||||
file that is inaccessible due to (e.g.) a missing "x" (search) permission
|
|
||||||
on a leading directory, then g_file_query_info() succeeds, our
|
|
||||||
"skipMissing" branch is dead, g_file_info_get_attribute_uint32() is
|
|
||||||
reached, and it returns G_FILE_TYPE_UNKNOWN.
|
|
||||||
|
|
||||||
As a consequence, the outer osinfo_loader_process_default_path() function
|
|
||||||
can fail, even though it passes skipMissing=TRUE to
|
|
||||||
osinfo_loader_process_list(). Example:
|
|
||||||
|
|
||||||
> $ HOME=/root \
|
|
||||||
> OSINFO_SYSTEM_DIR=/usr/share/osinfo \
|
|
||||||
> build/tools/osinfo-query os
|
|
||||||
> Error loading OS data: Can't read path /root/.config/osinfo
|
|
||||||
|
|
||||||
Arguably, this situation should be handled by simply skipping the
|
|
||||||
inaccessible path, as if all leading directories could be searched, and
|
|
||||||
only the last pathname compontent (the filename entry) didn't exist in its
|
|
||||||
direct parent directory.
|
|
||||||
|
|
||||||
The glib regression was reported in 2017:
|
|
||||||
|
|
||||||
https://bugzilla.gnome.org/show_bug.cgi?id=777187
|
|
||||||
|
|
||||||
and then migrated to gitlab:
|
|
||||||
|
|
||||||
https://gitlab.gnome.org/GNOME/glib/-/issues/1237
|
|
||||||
|
|
||||||
but it's still not solved today.
|
|
||||||
|
|
||||||
Work around the issue by honoring "skipMissing" on the G_FILE_TYPE_UNKNOWN
|
|
||||||
branch. Demonstration:
|
|
||||||
|
|
||||||
> $ HOME=/root \
|
|
||||||
> OSINFO_SYSTEM_DIR=/usr/share/osinfo \
|
|
||||||
> build/tools/osinfo-query os
|
|
||||||
>
|
|
||||||
> ** (osinfo-query:9924): WARNING **: 13:23:12.776: Can't read path /root/.config/osinfo
|
|
||||||
> Short ID | Name | Version | ID
|
|
||||||
> ----------------+------------------+---------+----------------------------------------
|
|
||||||
> alpinelinux3.5 | Alpine Linux 3.5 | 3.5 | http://alpinelinux.org/alpinelinux/3.5
|
|
||||||
> ...
|
|
||||||
|
|
||||||
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2051559
|
|
||||||
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
|
|
||||||
---
|
|
||||||
osinfo/osinfo_loader.c | 15 ++++++
|
|
||||||
tests/test-loader.c | 105 +++++++++++++++++++++++++++++++++++++++++
|
|
||||||
2 files changed, 120 insertions(+)
|
|
||||||
|
|
||||||
diff --git a/osinfo/osinfo_loader.c b/osinfo/osinfo_loader.c
|
|
||||||
index 96ca6ee..e244b3f 100644
|
|
||||||
--- a/osinfo/osinfo_loader.c
|
|
||||||
+++ b/osinfo/osinfo_loader.c
|
|
||||||
@@ -2377,6 +2377,21 @@ static void osinfo_loader_find_files(OsinfoLoader *loader,
|
|
||||||
} else if (type == G_FILE_TYPE_UNKNOWN) {
|
|
||||||
g_autofree gchar *path = g_file_get_path(file);
|
|
||||||
g_autofree gchar *msg = g_strdup_printf("Can't read path %s", path);
|
|
||||||
+ if (skipMissing) {
|
|
||||||
+ /* This is a work-around for
|
|
||||||
+ * <https://gitlab.gnome.org/GNOME/glib/-/issues/1237>. If the
|
|
||||||
+ * lstat() call underlying our g_file_query_info() call at the top
|
|
||||||
+ * of this function fails for "path" with EACCES, then
|
|
||||||
+ * g_file_query_info() should fail, and the "skipMissing" branch up
|
|
||||||
+ * there should suppress the error and return cleanly.
|
|
||||||
+ * Unfortunately, _g_local_file_info_get() masks the lstat()
|
|
||||||
+ * failure, g_file_info_get_attribute_uint32() is reached above,
|
|
||||||
+ * and returns G_FILE_TYPE_UNKNOWN for the file that could never be
|
|
||||||
+ * accessed. So we need to consider "skipMissing" here too.
|
|
||||||
+ */
|
|
||||||
+ g_warning("%s", msg);
|
|
||||||
+ return;
|
|
||||||
+ }
|
|
||||||
OSINFO_LOADER_SET_ERROR(&error, msg);
|
|
||||||
g_propagate_error(err, error);
|
|
||||||
} else {
|
|
||||||
diff --git a/tests/test-loader.c b/tests/test-loader.c
|
|
||||||
index 6644943..bb86585 100644
|
|
||||||
--- a/tests/test-loader.c
|
|
||||||
+++ b/tests/test-loader.c
|
|
||||||
@@ -16,6 +16,8 @@
|
|
||||||
*/
|
|
||||||
|
|
||||||
#include <osinfo/osinfo.h>
|
|
||||||
+#include <glib/gstdio.h>
|
|
||||||
+#include <unistd.h>
|
|
||||||
|
|
||||||
static void
|
|
||||||
test_basic(void)
|
|
||||||
@@ -31,6 +33,101 @@ test_basic(void)
|
|
||||||
g_object_unref(loader);
|
|
||||||
}
|
|
||||||
|
|
||||||
+typedef struct {
|
|
||||||
+ gchar *tmp_parent;
|
|
||||||
+ gchar *tmp_child;
|
|
||||||
+ gchar *orig_userdir;
|
|
||||||
+ gchar *expected_warning;
|
|
||||||
+} TestEaccesFixture;
|
|
||||||
+
|
|
||||||
+static void
|
|
||||||
+eacces_fixture_setup(TestEaccesFixture *fixture, gconstpointer user_data)
|
|
||||||
+{
|
|
||||||
+ gpointer rp;
|
|
||||||
+ gint ri;
|
|
||||||
+ gboolean rb;
|
|
||||||
+
|
|
||||||
+ /* create a temporary directory with permissions 0700 */
|
|
||||||
+ fixture->tmp_parent = g_strdup_printf("%s/%s", g_get_tmp_dir(),
|
|
||||||
+ "test_eacces.XXXXXX");
|
|
||||||
+ rp = g_mkdtemp_full(fixture->tmp_parent, 0700);
|
|
||||||
+ g_assert_nonnull(rp);
|
|
||||||
+
|
|
||||||
+ /* create a child directory called "osinfo" in it, with permissions 0700 */
|
|
||||||
+ fixture->tmp_child = g_strdup_printf("%s/osinfo", fixture->tmp_parent);
|
|
||||||
+ ri = g_mkdir(fixture->tmp_child, 0700);
|
|
||||||
+ g_assert_cmpint(ri, ==, 0);
|
|
||||||
+
|
|
||||||
+ /* revoke the search permission (0100) from the parent */
|
|
||||||
+ ri = g_chmod(fixture->tmp_parent, 0600);
|
|
||||||
+ g_assert_cmpint(ri, ==, 0);
|
|
||||||
+
|
|
||||||
+ /* stash the current value of OSINFO_USER_DIR */
|
|
||||||
+ fixture->orig_userdir = g_strdup(g_getenv("OSINFO_USER_DIR"));
|
|
||||||
+
|
|
||||||
+ /* point osinfo_loader_get_user_path() inside
|
|
||||||
+ * osinfo_loader_process_default_path() to the child directory
|
|
||||||
+ */
|
|
||||||
+ rb = g_setenv("OSINFO_USER_DIR", fixture->tmp_child, TRUE);
|
|
||||||
+ g_assert_true(rb);
|
|
||||||
+
|
|
||||||
+ /* format the pattern for the warning expected later on */
|
|
||||||
+ fixture->expected_warning = g_strdup_printf("Can't read path %s",
|
|
||||||
+ fixture->tmp_child);
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+static void
|
|
||||||
+eacces_fixture_teardown(TestEaccesFixture *fixture, gconstpointer user_data)
|
|
||||||
+{
|
|
||||||
+ gboolean rb;
|
|
||||||
+ gint ri;
|
|
||||||
+
|
|
||||||
+ /* free the expected warning pattern */
|
|
||||||
+ g_free(fixture->expected_warning);
|
|
||||||
+
|
|
||||||
+ /* restore the OSINFO_USER_DIR variable */
|
|
||||||
+ if (fixture->orig_userdir) {
|
|
||||||
+ rb = g_setenv("OSINFO_USER_DIR", fixture->orig_userdir, TRUE);
|
|
||||||
+ g_assert_true(rb);
|
|
||||||
+ g_free(fixture->orig_userdir);
|
|
||||||
+ } else {
|
|
||||||
+ g_unsetenv("OSINFO_USER_DIR");
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
+ /* restore search permission on the parent */
|
|
||||||
+ ri = g_chmod(fixture->tmp_parent, 0700);
|
|
||||||
+ g_assert_cmpint(ri, ==, 0);
|
|
||||||
+
|
|
||||||
+ /* remove both directories */
|
|
||||||
+ ri = g_rmdir(fixture->tmp_child);
|
|
||||||
+ g_assert_cmpint(ri, ==, 0);
|
|
||||||
+ g_free(fixture->tmp_child);
|
|
||||||
+
|
|
||||||
+ ri = g_rmdir(fixture->tmp_parent);
|
|
||||||
+ g_assert_cmpint(ri, ==, 0);
|
|
||||||
+ g_free(fixture->tmp_parent);
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
+static void
|
|
||||||
+test_eacces(TestEaccesFixture *fixture, gconstpointer user_data)
|
|
||||||
+{
|
|
||||||
+ OsinfoLoader *loader = osinfo_loader_new();
|
|
||||||
+ GError *error = NULL;
|
|
||||||
+
|
|
||||||
+ g_assert_true(OSINFO_IS_LOADER(loader));
|
|
||||||
+
|
|
||||||
+ /* this should trigger an EACCES in glib's lstat(), but not break db
|
|
||||||
+ * loading; also we expect the warning here
|
|
||||||
+ */
|
|
||||||
+ g_test_expect_message(G_LOG_DOMAIN, G_LOG_LEVEL_WARNING,
|
|
||||||
+ fixture->expected_warning);
|
|
||||||
+ osinfo_loader_process_default_path(loader, &error);
|
|
||||||
+ g_assert_no_error(error);
|
|
||||||
+ g_test_assert_expected_messages();
|
|
||||||
+
|
|
||||||
+ g_object_unref(loader);
|
|
||||||
+}
|
|
||||||
+
|
|
||||||
int
|
|
||||||
main(int argc, char *argv[])
|
|
||||||
{
|
|
||||||
@@ -38,6 +135,14 @@ main(int argc, char *argv[])
|
|
||||||
|
|
||||||
g_test_add_func("/loader/basic", test_basic);
|
|
||||||
|
|
||||||
+ /* the following test depends on a directory with file mode bits 0600 being
|
|
||||||
+ * unsearchable for the owner, so skip it if the test is running as root
|
|
||||||
+ */
|
|
||||||
+ if (geteuid() != 0) {
|
|
||||||
+ g_test_add("/loader/eacces", TestEaccesFixture, NULL,
|
|
||||||
+ eacces_fixture_setup, test_eacces, eacces_fixture_teardown);
|
|
||||||
+ }
|
|
||||||
+
|
|
||||||
/* Upfront so we don't confuse valgrind */
|
|
||||||
osinfo_entity_get_type();
|
|
||||||
osinfo_db_get_type();
|
|
||||||
--
|
|
||||||
2.34.1
|
|
||||||
|
|
||||||
Loading…
Reference in new issue