[PATCH gnome-team] Cogl: fix double free

  • Done
  • quality assurance status badge
Details
2 participants
  • Liliana Marie Prikler
  • Vivien Kraus
Owner
unassigned
Submitted by
Vivien Kraus
Severity
normal
V
V
Vivien Kraus wrote on 7 Sep 2023 07:36
(address . guix-patches@gnu.org)
03afaf481dfe74a9de14c77febf5d9241d0010e7.camel@planete-kraus.eu
* gnu/local.mk (dist_patch_DATA): Add cogl-fix-double-free.patch.
* gnu/packages/gnome.scm (cogl): Apply cogl-fix-double-free.patch.
* gnu/packages/patches/cogl-fix-double-free.patch: Avoid freeing the
dispaly used by a cogl context twice. Since the cogl repository is
read-only, this patch won’t be applied upstream.
---
gnu/local.mk | 1 +
gnu/packages/gnome.scm | 4 +++-
gnu/packages/patches/cogl-fix-double-free.patch | 12 ++++++++++++
3 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/cogl-fix-double-free.patch

Toggle diff (49 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 24fa8117c6..ae36dd49c9 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1024,6 +1024,7 @@ dist_patch_DATA = \
%D%/packages/patches/clucene-pkgconfig.patch \
%D%/packages/patches/cmake-curl-certificates-3.24.patch \
%D%/packages/patches/coda-use-system-libs.patch \
+ %D%/packages/patches/cogl-fix-double-free.patch \
%D%/packages/patches/collectd-5.11.0-noinstallvar.patch \
%D%/packages/patches/combinatorial-blas-awpm.patch \
%D%/packages/patches/combinatorial-blas-io-fix.patch \
diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 922d6ba24b..6b824c2832 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -6131,7 +6131,9 @@ (define-public cogl
(version-major+minor version) "/"
"cogl-" version ".tar.xz"))
(sha256
- (base32 "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))))
+ (base32 "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))
+ (patches
+ (search-patches "cogl-fix-double-free.patch"))))
;; NOTE: mutter exports a bundled fork of cogl, so when making changes to
;; cogl, corresponding changes may be appropriate in mutter as well.
(build-system gnu-build-system)
diff --git a/gnu/packages/patches/cogl-fix-double-free.patch b/gnu/packages/patches/cogl-fix-double-free.patch
new file mode 100644
index 0000000000..67391f6300
--- /dev/null
+++ b/gnu/packages/patches/cogl-fix-double-free.patch
@@ -0,0 +1,12 @@
+diff --git a/cogl-1.22.8/cogl/cogl-context.c b/cogl-1.22.8-fixed/cogl/cogl-context.c
+index a7eed29..2280942 100644
+--- a/cogl/cogl-context.c
++++ b/cogl/cogl-context.c
+@@ -219,6 +219,7 @@ cogl_context_new (CoglDisplay *display,
+ }
+
+ context->display = display;
++ cogl_object_ref (display);
+
+ /* This is duplicated data, but it's much more convenient to have
+ the driver attached to the context and the value is accessed a

base-commit: baf5b0745446dabe8166d860996dc54cfa09db3e
--
2.41.0
V
V
Vivien Kraus wrote on 7 Sep 2023 07:36
[PATCH gnome-team v2] Cogl: fix double free
(address . 65798@debbugs.gnu.org)
018c75fd44fa1b1462a49f5bf700b9194d9c6341.1694118524.git.vivien@planete-kraus.eu
* gnu/local.mk (dist_patch_DATA): Add cogl-fix-double-free.patch.
* gnu/packages/gnome.scm (cogl): Apply cogl-fix-double-free.patch.
* gnu/packages/patches/cogl-fix-double-free.patch: Avoid freeing the
dispaly used by a cogl context twice. Since the cogl repository is
read-only, this patch won’t be applied upstream.
---
gnu/local.mk | 1 +
gnu/packages/gnome.scm | 4 ++-
.../patches/cogl-fix-double-free.patch | 31 +++++++++++++++++++
3 files changed, 35 insertions(+), 1 deletion(-)
create mode 100644 gnu/packages/patches/cogl-fix-double-free.patch

Toggle diff (68 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 24fa8117c6..ae36dd49c9 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1024,6 +1024,7 @@ dist_patch_DATA = \
%D%/packages/patches/clucene-pkgconfig.patch \
%D%/packages/patches/cmake-curl-certificates-3.24.patch \
%D%/packages/patches/coda-use-system-libs.patch \
+ %D%/packages/patches/cogl-fix-double-free.patch \
%D%/packages/patches/collectd-5.11.0-noinstallvar.patch \
%D%/packages/patches/combinatorial-blas-awpm.patch \
%D%/packages/patches/combinatorial-blas-io-fix.patch \
diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 922d6ba24b..6b824c2832 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -6131,7 +6131,9 @@ (define-public cogl
(version-major+minor version) "/"
"cogl-" version ".tar.xz"))
(sha256
- (base32 "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))))
+ (base32 "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))
+ (patches
+ (search-patches "cogl-fix-double-free.patch"))))
;; NOTE: mutter exports a bundled fork of cogl, so when making changes to
;; cogl, corresponding changes may be appropriate in mutter as well.
(build-system gnu-build-system)
diff --git a/gnu/packages/patches/cogl-fix-double-free.patch b/gnu/packages/patches/cogl-fix-double-free.patch
new file mode 100644
index 0000000000..7094c4cf02
--- /dev/null
+++ b/gnu/packages/patches/cogl-fix-double-free.patch
@@ -0,0 +1,31 @@
+From 38d3fda8849ac327b473ac11dfac5499f595b7ac Mon Sep 17 00:00:00 2001
+Message-ID: <38d3fda8849ac327b473ac11dfac5499f595b7ac.1694118000.git.vivien@planete-kraus.eu>
+In-Reply-To: <cover.1694118000.git.vivien@planete-kraus.eu>
+References: <cover.1694118000.git.vivien@planete-kraus.eu>
+From: Vivien Kraus <vivien@planete-kraus.eu>
+Date: Thu, 7 Sep 2023 22:16:48 +0200
+Subject: [PATCH 1/1] Prevent double free on context objects
+
+The display is unrefed in the context destructor, but not refed in the
+constructor.
+
+This targets an archived (read-only) repository.
+---
+ cogl/cogl-context.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c
+index a7eed29a..22809424 100644
+--- a/cogl/cogl-context.c
++++ b/cogl/cogl-context.c
+@@ -219,6 +219,7 @@ cogl_context_new (CoglDisplay *display,
+ }
+
+ context->display = display;
++ cogl_object_ref (display);
+
+ /* This is duplicated data, but it's much more convenient to have
+ the driver attached to the context and the value is accessed a
+--
+2.41.0
+

base-commit: baf5b0745446dabe8166d860996dc54cfa09db3e
--
2.41.0
L
L
Liliana Marie Prikler wrote on 8 Sep 2023 06:24
db73ecb3e896b5d9faaf4c187c262ff7ce98beca.camel@gmail.com
Am Donnerstag, dem 07.09.2023 um 07:36 +0200 schrieb Vivien Kraus:
Toggle quote (4 lines)
> * gnu/local.mk (dist_patch_DATA): Add cogl-fix-double-free.patch.
> * gnu/packages/gnome.scm (cogl): Apply cogl-fix-double-free.patch.
> * gnu/packages/patches/cogl-fix-double-free.patch: Avoid freeing the
> dispaly used by a cogl context twice.
display

Usually you do (patch, register, use) in this order, but I can rewrite
the ChangeLog for you easily :)

Toggle quote (2 lines)
> Since the cogl repository is
> read-only, this patch won’t be applied upstream.
This information is not necessary in the ChangeLog. You could mention
it before that, but I think keeping it in the patch itself is a better
idea.

Toggle quote (40 lines)
> ---
>  gnu/local.mk                                  |  1 +
>  gnu/packages/gnome.scm                        |  4 ++-
>  .../patches/cogl-fix-double-free.patch        | 31
> +++++++++++++++++++
>  3 files changed, 35 insertions(+), 1 deletion(-)
>  create mode 100644 gnu/packages/patches/cogl-fix-double-free.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 24fa8117c6..ae36dd49c9 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1024,6 +1024,7 @@ dist_patch_DATA
> =                                         \
>    %D%/packages/patches/clucene-pkgconfig.patch                 \
>    %D%/packages/patches/cmake-curl-certificates-3.24.patch      \
>    %D%/packages/patches/coda-use-system-libs.patch              \
> +  %D%/packages/patches/cogl-fix-double-free.patch              \
>    %D%/packages/patches/collectd-5.11.0-
> noinstallvar.patch              \
>    %D%/packages/patches/combinatorial-blas-awpm.patch           \
>    %D%/packages/patches/combinatorial-blas-io-fix.patch         \
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index 922d6ba24b..6b824c2832 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -6131,7 +6131,9 @@ (define-public cogl
>                             (version-major+minor version) "/"
>                             "cogl-" version ".tar.xz"))
>         (sha256
> -        (base32
> "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))))
> +        (base32
> "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))
> +       (patches
> +        (search-patches "cogl-fix-double-free.patch"))))
>      ;; NOTE: mutter exports a bundled fork of cogl, so when making
> changes to
>      ;; cogl, corresponding changes may be appropriate in mutter as
> well.
This comment no longer applies to current mutter, for it has diverged
far enough.
Toggle quote (37 lines)
>      (build-system gnu-build-system)
> diff --git a/gnu/packages/patches/cogl-fix-double-free.patch
> b/gnu/packages/patches/cogl-fix-double-free.patch
> new file mode 100644
> index 0000000000..7094c4cf02
> --- /dev/null
> +++ b/gnu/packages/patches/cogl-fix-double-free.patch
> @@ -0,0 +1,31 @@
> +From 38d3fda8849ac327b473ac11dfac5499f595b7ac Mon Sep 17 00:00:00
> 2001
> +Message-ID:
> <38d3fda8849ac327b473ac11dfac5499f595b7ac.1694118000.git.vivien@plane
> te-kraus.eu>
> +In-Reply-To: <cover.1694118000.git.vivien@planete-kraus.eu>
> +References: <cover.1694118000.git.vivien@planete-kraus.eu>
> +From: Vivien Kraus <vivien@planete-kraus.eu>
> +Date: Thu, 7 Sep 2023 22:16:48 +0200
> +Subject: [PATCH 1/1] Prevent double free on context objects
> +
> +The display is unrefed in the context destructor, but not refed in
> the
> +constructor.
> +
> +This targets an archived (read-only) repository.
> +---
> + cogl/cogl-context.c | 1 +
> + 1 file changed, 1 insertion(+)
> +
> +diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c
> +index a7eed29a..22809424 100644
> +--- a/cogl/cogl-context.c
> ++++ b/cogl/cogl-context.c
> +@@ -219,6 +219,7 @@ cogl_context_new (CoglDisplay *display,
> +     }
> +
> +   context->display = display;
> ++  cogl_object_ref (display);
You can use context->display = cogl_object_ref (display) if it works
like g_object_ref.
Toggle quote (8 lines)
> +
> +   /* This is duplicated data, but it's much more convenient to have
> +      the driver attached to the context and the value is accessed a
> +--
> +2.41.0
> +
>
> base-commit: baf5b0745446dabe8166d860996dc54cfa09db3e
Will see what CI has to say, otherwise LGTM.

Cheers
V
V
Vivien Kraus wrote on 7 Sep 2023 07:36
[gnome-team v3] Cogl: fix double free
(address . 65798@debbugs.gnu.org)
e79232881a6dcc573a730ff8b66baddb6891ea1c.1694149645.git.vivien@planete-kraus.eu
* gnu/packages/patches/cogl-fix-double-free.patch: Avoid freeing the
dispaly used by a cogl context twice.
* gnu/local.mk (dist_patch_DATA): Add cogl-fix-double-free.patch.
* gnu/packages/gnome.scm (cogl): Apply cogl-fix-double-free.patch. Remove the
comment about the changes to be reflected in mutter, as their bundled cogl has
diverged far enough.
---
gnu/local.mk | 1 +
gnu/packages/gnome.scm | 6 ++--
.../patches/cogl-fix-double-free.patch | 32 +++++++++++++++++++
3 files changed, 36 insertions(+), 3 deletions(-)
create mode 100644 gnu/packages/patches/cogl-fix-double-free.patch

Toggle diff (71 lines)
diff --git a/gnu/local.mk b/gnu/local.mk
index 24fa8117c6..ae36dd49c9 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -1024,6 +1024,7 @@ dist_patch_DATA = \
%D%/packages/patches/clucene-pkgconfig.patch \
%D%/packages/patches/cmake-curl-certificates-3.24.patch \
%D%/packages/patches/coda-use-system-libs.patch \
+ %D%/packages/patches/cogl-fix-double-free.patch \
%D%/packages/patches/collectd-5.11.0-noinstallvar.patch \
%D%/packages/patches/combinatorial-blas-awpm.patch \
%D%/packages/patches/combinatorial-blas-io-fix.patch \
diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
index 922d6ba24b..894aac8202 100644
--- a/gnu/packages/gnome.scm
+++ b/gnu/packages/gnome.scm
@@ -6131,9 +6131,9 @@ (define-public cogl
(version-major+minor version) "/"
"cogl-" version ".tar.xz"))
(sha256
- (base32 "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))))
- ;; NOTE: mutter exports a bundled fork of cogl, so when making changes to
- ;; cogl, corresponding changes may be appropriate in mutter as well.
+ (base32 "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))
+ (patches
+ (search-patches "cogl-fix-double-free.patch"))))
(build-system gnu-build-system)
(native-inputs
`(("glib:bin" ,glib "bin") ; for glib-mkenums
diff --git a/gnu/packages/patches/cogl-fix-double-free.patch b/gnu/packages/patches/cogl-fix-double-free.patch
new file mode 100644
index 0000000000..e7a994b33a
--- /dev/null
+++ b/gnu/packages/patches/cogl-fix-double-free.patch
@@ -0,0 +1,32 @@
+From 15d0f7d96cf53263196e26f2eb48ededdff0efeb Mon Sep 17 00:00:00 2001
+Message-ID: <15d0f7d96cf53263196e26f2eb48ededdff0efeb.1694148833.git.vivien@planete-kraus.eu>
+From: Vivien Kraus <vivien@planete-kraus.eu>
+Date: Thu, 7 Sep 2023 22:16:48 +0200
+Subject: [PATCH] Prevent double free on context objects
+
+The display is unrefed in the context destructor, but not refed in the
+constructor.
+
+This targets an archived (read-only) repository.
+---
+ cogl/cogl-context.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c
+index a7eed29a..7cdc9fe7 100644
+--- a/cogl/cogl-context.c
++++ b/cogl/cogl-context.c
+@@ -218,7 +218,7 @@ cogl_context_new (CoglDisplay *display,
+ return NULL;
+ }
+
+- context->display = display;
++ context->display = cogl_object_ref (display);
+
+ /* This is duplicated data, but it's much more convenient to have
+ the driver attached to the context and the value is accessed a
+
+base-commit: 61d966c7442d521e38572b7f93ac7b8973a9c65e
+--
+2.41.0
+

base-commit: baf5b0745446dabe8166d860996dc54cfa09db3e
--
2.41.0
L
L
Liliana Marie Prikler wrote on 8 Sep 2023 18:59
b0055d7805af932b5076b244cf205c0d2bd303bf.camel@gmail.com
Am Donnerstag, dem 07.09.2023 um 07:36 +0200 schrieb Vivien Kraus:
Toggle quote (2 lines)
> * gnu/packages/patches/cogl-fix-double-free.patch: Avoid freeing the
> dispaly used by a cogl context twice.
To whom commits this (likely me in the future): s/dispaly/display/
Toggle quote (6 lines)
> * gnu/local.mk (dist_patch_DATA): Add cogl-fix-double-free.patch.
> * gnu/packages/gnome.scm (cogl): Apply cogl-fix-double-free.patch.
> Remove the
> comment about the changes to be reflected in mutter, as their bundled
> cogl has
> diverged far enough.
I'll also add a blurb before the ChangeLog and shorten this to make the
change clearer.
Toggle quote (88 lines)
> ---
>  gnu/local.mk                                  |  1 +
>  gnu/packages/gnome.scm                        |  6 ++--
>  .../patches/cogl-fix-double-free.patch        | 32
> +++++++++++++++++++
>  3 files changed, 36 insertions(+), 3 deletions(-)
>  create mode 100644 gnu/packages/patches/cogl-fix-double-free.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 24fa8117c6..ae36dd49c9 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1024,6 +1024,7 @@ dist_patch_DATA
> =                                         \
>    %D%/packages/patches/clucene-pkgconfig.patch                 \
>    %D%/packages/patches/cmake-curl-certificates-3.24.patch      \
>    %D%/packages/patches/coda-use-system-libs.patch              \
> +  %D%/packages/patches/cogl-fix-double-free.patch              \
>    %D%/packages/patches/collectd-5.11.0-
> noinstallvar.patch              \
>    %D%/packages/patches/combinatorial-blas-awpm.patch           \
>    %D%/packages/patches/combinatorial-blas-io-fix.patch         \
> diff --git a/gnu/packages/gnome.scm b/gnu/packages/gnome.scm
> index 922d6ba24b..894aac8202 100644
> --- a/gnu/packages/gnome.scm
> +++ b/gnu/packages/gnome.scm
> @@ -6131,9 +6131,9 @@ (define-public cogl
>                             (version-major+minor version) "/"
>                             "cogl-" version ".tar.xz"))
>         (sha256
> -        (base32
> "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))))
> -    ;; NOTE: mutter exports a bundled fork of cogl, so when making
> changes to
> -    ;; cogl, corresponding changes may be appropriate in mutter as
> well.
> +        (base32
> "0nfph4ai60ncdx7hy6hl1i1cmp761jgnyjfhagzi0iqq36qb41d8"))
> +       (patches
> +        (search-patches "cogl-fix-double-free.patch"))))
>      (build-system gnu-build-system)
>      (native-inputs
>       `(("glib:bin" ,glib "bin")     ; for glib-mkenums
> diff --git a/gnu/packages/patches/cogl-fix-double-free.patch
> b/gnu/packages/patches/cogl-fix-double-free.patch
> new file mode 100644
> index 0000000000..e7a994b33a
> --- /dev/null
> +++ b/gnu/packages/patches/cogl-fix-double-free.patch
> @@ -0,0 +1,32 @@
> +From 15d0f7d96cf53263196e26f2eb48ededdff0efeb Mon Sep 17 00:00:00
> 2001
> +Message-ID:
> <15d0f7d96cf53263196e26f2eb48ededdff0efeb.1694148833.git.vivien@plane
> te-kraus.eu>
> +From: Vivien Kraus <vivien@planete-kraus.eu>
> +Date: Thu, 7 Sep 2023 22:16:48 +0200
> +Subject: [PATCH] Prevent double free on context objects
> +
> +The display is unrefed in the context destructor, but not refed in
> the
> +constructor.
> +
> +This targets an archived (read-only) repository.
> +---
> + cogl/cogl-context.c | 2 +-
> + 1 file changed, 1 insertion(+), 1 deletion(-)
> +
> +diff --git a/cogl/cogl-context.c b/cogl/cogl-context.c
> +index a7eed29a..7cdc9fe7 100644
> +--- a/cogl/cogl-context.c
> ++++ b/cogl/cogl-context.c
> +@@ -218,7 +218,7 @@ cogl_context_new (CoglDisplay *display,
> +       return NULL;
> +     }
> +
> +-  context->display = display;
> ++  context->display = cogl_object_ref (display);
> +
> +   /* This is duplicated data, but it's much more convenient to have
> +      the driver attached to the context and the value is accessed a
> +
> +base-commit: 61d966c7442d521e38572b7f93ac7b8973a9c65e
> +--
> +2.41.0
> +
>
> base-commit: baf5b0745446dabe8166d860996dc54cfa09db3e
LGTM, now waiting for CI to build it.

Cheers
L
L
Liliana Marie Prikler wrote on 15 Sep 2023 18:17
ef12ccfc13a8a3133802030436b69425e5587854.camel@gmail.com
Am Freitag, dem 08.09.2023 um 18:59 +0200 schrieb Liliana Marie
Prikler:
Toggle quote (1 lines)
> LGTM, now waiting for CI to build it.
QA green, it's merge time.

Thanks
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 65798@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 65798
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch