[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1071159: bookworm-pu: package glib2.0/2.74.6-2+deb12u3



Package: release.debian.org
Severity: normal
Tags: bookworm
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: glib2.0@packages.debian.org
Control: affects -1 + src:glib2.0

[ Reason ]
Fix a minor memory leak introduced by recent security updates.

[ Impact ]
In an unusual situation that I believe is very rare in practice, programs
using D-Bus via GLib will leak memory.

(Specifically, that situation is: the program allocates a GDBus message
with a non-empty body, then replaces the message body with something
different, and the original body is leaked. The only use I'm aware of
for editing messages in this way in Debian was in a hack to avoid gdm3
upgrades from jessie to stretch being unable to unlock the screensaver,
by rewriting D-Bus messages in-place, and that was removed between
stretch and buster.)

[ Tests ]
There is a relatively extensive test suite, which is how the leak was found
in the first place, and it still passes.

The proposed version is also working well to run the GNOME environment
where I'm typing this.

[ Risks ]
Low risk. The change is small and obviously correct, already migrated
to testing, and was included in the backported security fix for Debian
10 LTS. It was discovered too late to be included with the more serious
regression fixes in Debian 12 and 11, and in any case would not have been
urgent enough to justify delaying fixes for the more serious regression.

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in (old)stable
      - the debdiff is vs. bookworm-security, I assume that's OK
  [x] the issue is verified as fixed in unstable

[ Changes ]
All changes are for this single bug fix.

[ Other info ]
I already uploaded to -proposed-updates.

The security team agreed with my assessment that this is not important
enough to issue another DSA update.

Thanks,
    smcv
diffstat for glib2.0-2.74.6 glib2.0-2.74.6

 debian/changelog                                                               |    8 +
 debian/patches/gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch |   41 ++++++++++
 debian/patches/series                                                          |    1 
 gio/gdbusmessage.c                                                             |    6 -
 4 files changed, 53 insertions(+), 3 deletions(-)

diff -Nru glib2.0-2.74.6/debian/changelog glib2.0-2.74.6/debian/changelog
--- glib2.0-2.74.6/debian/changelog	2024-05-08 12:35:38.000000000 +0100
+++ glib2.0-2.74.6/debian/changelog	2024-05-14 11:11:32.000000000 +0100
@@ -1,3 +1,11 @@
+glib2.0 (2.74.6-2+deb12u3) bookworm; urgency=medium
+
+  * d/p/gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch:
+    Add patch from upstream fixing a memory leak that can occur in
+    rare situations since 2.74.6-2+deb12u1 (Closes: #1070851)
+
+ -- Simon McVittie <smcv@debian.org>  Tue, 14 May 2024 11:11:32 +0100
+
 glib2.0 (2.74.6-2+deb12u2) bookworm-security; urgency=high
 
   * d/p/CVE-2024-34397/gdbusconnection-Allow-name-owners-to-have-the-syntax-of-a.patch:
diff -Nru glib2.0-2.74.6/debian/patches/gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch glib2.0-2.74.6/debian/patches/gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch
--- glib2.0-2.74.6/debian/patches/gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch	1970-01-01 01:00:00.000000000 +0100
+++ glib2.0-2.74.6/debian/patches/gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch	2024-05-14 11:11:32.000000000 +0100
@@ -0,0 +1,41 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <mail@3v1n0.net>
+Date: Wed, 8 May 2024 22:53:51 +0200
+Subject: gdbusmessage: Clean the cached arg0 when setting the message body
+
+We're now caching arg0 but such value is not cleared when a new body is
+set as it's in the connection filter test cases where we've a leak as
+highlighted by both valgrind and leak sanitizer
+
+Origin: upstream, 2.80.3, commit:fe89e9f3cb6e0fd0dc2bd8a2d413799e1443cef1
+Bug-Debian: https://bugs.debian.org/1070851
+---
+ gio/gdbusmessage.c | 6 +++---
+ 1 file changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/gio/gdbusmessage.c b/gio/gdbusmessage.c
+index a8656df..9e8fca7 100644
+--- a/gio/gdbusmessage.c
++++ b/gio/gdbusmessage.c
+@@ -1164,10 +1164,12 @@ g_dbus_message_set_body (GDBusMessage  *message,
+ 
+   if (message->body != NULL)
+     g_variant_unref (message->body);
++
++  g_clear_pointer (&message->arg0_cache, g_variant_unref);
++
+   if (body == NULL)
+     {
+       message->body = NULL;
+-      message->arg0_cache = NULL;
+       g_dbus_message_set_signature (message, NULL);
+     }
+   else
+@@ -1181,8 +1183,6 @@ g_dbus_message_set_body (GDBusMessage  *message,
+       if (g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
+           g_variant_n_children (message->body) > 0)
+         message->arg0_cache = g_variant_get_child_value (message->body, 0);
+-      else
+-        message->arg0_cache = NULL;
+ 
+       type_string = g_variant_get_type_string (body);
+       type_string_len = strlen (type_string);
diff -Nru glib2.0-2.74.6/debian/patches/series glib2.0-2.74.6/debian/patches/series
--- glib2.0-2.74.6/debian/patches/series	2024-05-08 12:35:38.000000000 +0100
+++ glib2.0-2.74.6/debian/patches/series	2024-05-14 11:11:32.000000000 +0100
@@ -42,3 +42,4 @@
 CVE-2024-34397/tests-Ensure-that-unsubscribing-with-GetNameOwner-in-flig.patch
 CVE-2024-34397/gdbus-proxy-test-Wait-before-asserting-name-owner-has-gon.patch
 CVE-2024-34397/gdbusconnection-Allow-name-owners-to-have-the-syntax-of-a.patch
+gdbusmessage-Clean-the-cached-arg0-when-setting-the-messa.patch
diff -Nru glib2.0-2.74.6/gio/gdbusmessage.c glib2.0-2.74.6/gio/gdbusmessage.c
--- glib2.0-2.74.6/gio/gdbusmessage.c	2024-05-14 20:20:36.000000000 +0100
+++ glib2.0-2.74.6/gio/gdbusmessage.c	2024-05-14 20:20:36.000000000 +0100
@@ -1164,10 +1164,12 @@
 
   if (message->body != NULL)
     g_variant_unref (message->body);
+
+  g_clear_pointer (&message->arg0_cache, g_variant_unref);
+
   if (body == NULL)
     {
       message->body = NULL;
-      message->arg0_cache = NULL;
       g_dbus_message_set_signature (message, NULL);
     }
   else
@@ -1181,8 +1183,6 @@
       if (g_variant_is_of_type (message->body, G_VARIANT_TYPE_TUPLE) &&
           g_variant_n_children (message->body) > 0)
         message->arg0_cache = g_variant_get_child_value (message->body, 0);
-      else
-        message->arg0_cache = NULL;
 
       type_string = g_variant_get_type_string (body);
       type_string_len = strlen (type_string);

Reply to: