From e9818571e3065ae85ba222b447ad4ad9b80994a8 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 18 Oct 2023 16:03:21 +0900 Subject: [PATCH 1/6] secret-paths: Fix memleak when unlocking a path A GPtrArray allocated to temporarily hold (un)locked item paths was not freed when the collection has a non-empty D-Bus path. Signed-off-by: Daiki Ueno --- libsecret/secret-paths.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libsecret/secret-paths.c b/libsecret/secret-paths.c index fdd0580..577ea56 100644 --- a/libsecret/secret-paths.c +++ b/libsecret/secret-paths.c @@ -1044,7 +1044,6 @@ on_xlock_called (GObject *source, XlockClosure *closure = g_task_get_task_data (task); GCancellable *cancellable = g_task_get_cancellable (task); SecretService *self = SECRET_SERVICE (g_task_get_source_object (task)); - GPtrArray *xlocked_array; const gchar *prompt = NULL; gchar **xlocked = NULL; GError *error = NULL; @@ -1056,11 +1055,13 @@ on_xlock_called (GObject *source, g_task_return_error (task, g_steal_pointer (&error)); } else { - xlocked_array = g_ptr_array_new_with_free_func (g_free); - g_variant_get (retval, "(^ao&o)", &xlocked, &prompt); if (_secret_util_empty_path (prompt)) { + GPtrArray *xlocked_array; + + xlocked_array = g_ptr_array_new_with_free_func (g_free); + for (i = 0; xlocked[i]; i++) g_ptr_array_add (xlocked_array, g_strdup (xlocked[i])); From 92705b58a32ee18eb3891872c98cf85a316b62c4 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 19 Oct 2023 13:50:12 +0900 Subject: [PATCH 2/6] secret-paths: Make sure to unref GVariant The GVariant returned in secret_service_get_secret{,s}_for_dbus_path{,s}_finish should be unref'ed after use. Signed-off-by: Daiki Ueno --- libsecret/secret-paths.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/libsecret/secret-paths.c b/libsecret/secret-paths.c index 577ea56..f837bb2 100644 --- a/libsecret/secret-paths.c +++ b/libsecret/secret-paths.c @@ -783,6 +783,7 @@ secret_service_get_secret_for_dbus_path_finish (SecretService *self, GError **error) { GVariant *ret; + SecretValue *value; g_return_val_if_fail (SECRET_IS_SERVICE (self), NULL); g_return_val_if_fail (g_task_is_valid (result, self), NULL); @@ -796,7 +797,9 @@ secret_service_get_secret_for_dbus_path_finish (SecretService *self, return NULL; } - return _secret_service_decode_get_secrets_first (self, ret); + value = _secret_service_decode_get_secrets_first (self, ret); + g_variant_unref (ret); + return value; } /** @@ -920,6 +923,7 @@ secret_service_get_secrets_for_dbus_paths_finish (SecretService *self, GError **error) { GVariant *ret; + GHashTable *values; g_return_val_if_fail (SECRET_IS_SERVICE (self), NULL); g_return_val_if_fail (g_task_is_valid (result, self), NULL); @@ -933,7 +937,9 @@ secret_service_get_secrets_for_dbus_paths_finish (SecretService *self, return NULL; } - return _secret_service_decode_get_secrets_all (self, ret); + values = _secret_service_decode_get_secrets_all (self, ret); + g_variant_unref (ret); + return values; } /** From f83cd26858bb2e6dd15c7be29f5e27615ed66dd6 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 19 Oct 2023 13:47:20 +0900 Subject: [PATCH 3/6] secret-service: Don't unnecessary increase refcount As the GVariant returned in secret_service_real_prompt_finish should be already sunk by secret_prompt_perform_finish, calling g_variant_ref_sink actually increases the refcount and causes a leak. Signed-off-by: Daiki Ueno --- libsecret/secret-service.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsecret/secret-service.c b/libsecret/secret-service.c index 6b3a743..909ab37 100644 --- a/libsecret/secret-service.c +++ b/libsecret/secret-service.c @@ -372,7 +372,7 @@ secret_service_real_prompt_finish (SecretService *self, return NULL; } - return g_variant_ref_sink (retval); + return retval; } static void From 3c975876088a9220c0e1ecea903fc74af8e62107 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Thu, 19 Oct 2023 17:22:13 +0900 Subject: [PATCH 4/6] secret-file-collection: Improve etag tracking This resets self->etag only after successful load of the contents, by using a temporary variable and checking error of g_file_replace_contents_finish, etc. Signed-off-by: Daiki Ueno --- libsecret/secret-file-collection.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/libsecret/secret-file-collection.c b/libsecret/secret-file-collection.c index 1819e83..96f8226 100644 --- a/libsecret/secret-file-collection.c +++ b/libsecret/secret-file-collection.c @@ -430,13 +430,16 @@ ensure_up_to_date (SecretFileCollection *self) gsize length = 0; gboolean success; GError *error = NULL; + gchar *etag = NULL; self->file_last_modified = last_modified; - g_clear_pointer (&self->etag, g_free); - success = g_file_load_contents (self->file, NULL, &contents, &length, &self->etag, &error); + success = g_file_load_contents (self->file, NULL, &contents, &length, &etag, &error); - if (!success && g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { + if (success) { + g_clear_pointer (&self->etag, g_free); + self->etag = g_steal_pointer (&etag); + } else if (g_error_matches (error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND)) { g_clear_error (&error); success = init_empty_file (self, &error); @@ -464,12 +467,13 @@ on_load_contents (GObject *source_object, gsize length; GError *error = NULL; gboolean ret; + gchar *etag = NULL; self->file_last_modified = get_file_last_modified (self); ret = g_file_load_contents_finish (file, result, &contents, &length, - &self->etag, + &etag, &error); if (!ret) { @@ -488,6 +492,9 @@ on_load_contents (GObject *source_object, return; } + g_clear_pointer (&self->etag, g_free); + self->etag = g_steal_pointer (&etag); + ret = load_contents (self, contents, length, &error); if (ret) g_task_return_boolean (task, ret); @@ -864,14 +871,17 @@ on_replace_contents (GObject *source_object, GTask *task = G_TASK (user_data); SecretFileCollection *self = g_task_get_source_object (task); GError *error = NULL; + gchar *etag = NULL; - if (!g_file_replace_contents_finish (file, result, &self->etag, &error)) { + if (!g_file_replace_contents_finish (file, result, &etag, &error)) { g_task_return_error (task, error); g_object_unref (task); return; } self->file_last_modified = get_file_last_modified (self); + g_clear_pointer (&self->etag, g_free); + self->etag = g_steal_pointer (&etag); g_task_return_boolean (task, TRUE); g_object_unref (task); From 7f97e5e0fa3f56b4f38efb142a28f687dea16529 Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 18 Oct 2023 16:08:58 +0900 Subject: [PATCH 5/6] .gitlab-ci.yml: Add LSan suppressions file This adds a suppression file for Leak Sanitizer to ignore known leaks in libgio-2.0.so. Signed-off-by: Daiki Ueno --- .gitlab-ci.yml | 1 + build/lsan.supp | 2 ++ 2 files changed, 3 insertions(+) create mode 100644 build/lsan.supp diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 27eafb1..35c1f1b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -26,6 +26,7 @@ fedora:asan: before_script: - dbus-uuidgen --ensure script: + - export LSAN_OPTIONS=suppressions=$PWD/build/lsan.supp - meson _build -Db_sanitize=address -Dgtk_doc=false -Dintrospection=false - meson compile -C _build - eval `dbus-launch --sh-syntax` diff --git a/build/lsan.supp b/build/lsan.supp new file mode 100644 index 0000000..665a172 --- /dev/null +++ b/build/lsan.supp @@ -0,0 +1,2 @@ +# https://gitlab.gnome.org/GNOME/glib/-/issues/2312 +leak:async_initable_init_first From 8efde5045581e041518b05006468f0cdd6db592d Mon Sep 17 00:00:00 2001 From: Daiki Ueno Date: Wed, 18 Oct 2023 16:17:09 +0900 Subject: [PATCH 6/6] .gitlab-ci: Update CI base image to Fedora 38 Signed-off-by: Daiki Ueno --- .gitlab-ci.yml | 3 +-- .gitlab-ci/master.Dockerfile | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 35c1f1b..da00840 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: registry.gitlab.gnome.org/gnome/libsecret/master:v1 +image: registry.gitlab.gnome.org/gnome/libsecret/master:v2 stages: - build @@ -31,7 +31,6 @@ fedora:asan: - meson compile -C _build - eval `dbus-launch --sh-syntax` - meson test -C _build --print-errorlogs - allow_failure: true artifacts: when: on_failure paths: diff --git a/.gitlab-ci/master.Dockerfile b/.gitlab-ci/master.Dockerfile index ee0c344..56c8672 100644 --- a/.gitlab-ci/master.Dockerfile +++ b/.gitlab-ci/master.Dockerfile @@ -1,4 +1,4 @@ -FROM fedora:34 +FROM fedora:38 RUN dnf update -y \ && dnf install -y \