[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

Debbugs page

Vivien Kraus wrote 2 years ago
(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
Vivien Kraus wrote 2 years ago
[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
Liliana Marie Prikler wrote 2 years ago
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
Vivien Kraus wrote 2 years ago
[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
Liliana Marie Prikler wrote 2 years ago
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
Liliana Marie Prikler wrote 1 years ago
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
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help