Re: Smooth gdm -> GNOME3 session transition broken with 1.20 modesetting driver
Hi Hans, On Wed, 5 Sep 2018 at 19:23, Hans de Goede wrote: > Under Fedora 29 (xserver-1.20) the transition from GDM to > the GNOME3 session is no longer smooth, it seems that the > screen is cleared to black when the Xserver starts instead > of inheriting the framebuffer contents from GDM as before. > > Changing the DDX driver from modesetting to intel fixes this, > I think this may be caused by the new atomic support in the > modesetting driver. It's caused by support for modifiers: this allows Mesa to use a multi-planar framebuffer (auxiliary compression plane), which the new X server can't then read back from because drmModeGetFB only supports a single plane. I've written up the patches to do that[0], but last time I tried to respin the current mainline was broken and didn't actually boot. I've actually just picked that up this week and am planning to send a new series out tomorrow. [0]: https://lists.freedesktop.org/archives/xorg-devel/2018-March/056363.html Cheers, Daniel ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Smooth gdm -> GNOME3 session transition broken with 1.20 modesetting driver
Hi All, Under Fedora 29 (xserver-1.20) the transition from GDM to the GNOME3 session is no longer smooth, it seems that the screen is cleared to black when the Xserver starts instead of inheriting the framebuffer contents from GDM as before. Changing the DDX driver from modesetting to intel fixes this, I think this may be caused by the new atomic support in the modesetting driver. Note if you try to reproduce this on Fedora 29 recent gdm versions also have some issues (*) which cause the transition to be non smooth it is best to downgrade gdm to 3.28.x (from koji) and then try with the intel ddx vs the modesetting ddx to clearly see the difference. Regards, Hans *) These are mostly fixed, but not entirely yet, see: https://gitlab.gnome.org/GNOME/gdm/merge_requests/45 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: WebKit failing to find GLXFBConfig, confusion around fbconfigs + swrast
Hi Daniel, On 27 August 2018 at 09:07, Daniel Drake wrote: > Questions: > > 1. What should webkit be doing in event of it not being to find a > GLXFBConfig that corresponds to the X visual of it's window? > > Attempt another config that user(webkit) knows how to work with? > 2. Why is swrast coming into the picture? Is swrast being used for rendering? > You're using the modesetting DDX, so the 2D acceleration comes from glamor/OpenGL. When the driver name cannot be retrieved, glamor will use ... well no driver - aka swrast. Possible solutions/workarounds: - try the intel ddx - not everyone is using it, it sees development yet no releases :-\ - audit the codepaths if one cannot get the driver name otherwise > 4. Why is there still a list of PCI IDs in the X server? > Nobody had the time/interest to fix that up. I did fold the 5 (IIRC) different codepaths in Mesa down to 1. ETIME and EWORK kind of got in the way of fixing up the X server. I guess it could go up my priority list, if employer (Collabora) suggests it ;-) HTH Emil ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] glx: check for indirect context in CreateContextAttribsARB()
Commit 99f0365b "Add a command line argument for disabling indirect GLX"
added a test to check if indirect context are enabled in
`DoCreateContext()` but `__glXDisp_CreateContextAttribsARB()` doesn't
use `DoCreateContext()` and doesn't check if indirect context is
enabled.
As a result, clients can still manage to create indirect contexts using
`glXCreateContextAttribsARB()` even if indirect contexts are disabled,
which can possibly crash Xservers such as Xwayland or Xephyr when the
context is destroyed.
To avoid the issue, check for `enableIndirectGLX` in
`__glXDisp_CreateContextAttribsARB()` as well.
Fixes: 99f0365b "Add a command line argument for disabling indirect GLX"
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107508
Signed-off-by: Olivier Fourdan
---
glx/createcontext.c | 12
1 file changed, 12 insertions(+)
diff --git a/glx/createcontext.c b/glx/createcontext.c
index 7d09c3a1c..24b02ddfb 100644
--- a/glx/createcontext.c
+++ b/glx/createcontext.c
@@ -28,6 +28,7 @@
#include "glxserver.h"
#include "glxext.h"
#include "indirect_dispatch.h"
+#include "opaque.h"
#define ALL_VALID_FLAGS \
(GLX_CONTEXT_DEBUG_BIT_ARB | GLX_CONTEXT_FORWARD_COMPATIBLE_BIT_ARB \
@@ -320,6 +321,17 @@ __glXDisp_CreateContextAttribsARB(__GLXclientState * cl,
GLbyte * pc)
err = BadAlloc;
}
else {
+/* Only allow creating indirect GLX contexts if allowed by
+ * server command line. Indirect GLX is of limited use (since
+ * it's only GL 1.4), it's slower than direct contexts, and
+ * it's a massive attack surface for buffer overflow type
+ * errors.
+ */
+if (!enableIndirectGLX) {
+client->errorValue = req->isDirect;
+return BadValue;
+}
+
ctx = glxScreen->createContext(glxScreen, config, shareCtx,
req->numAttribs, (uint32_t *) attribs,
);
--
2.17.1
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH v2] present: fix freed pointer access
Hi,
On Tue, Sep 4, 2018 at 6:28 PM Lionel Landwerlin
wrote:
>
> Oh well...
> I'm sure you'll be able to fix it faster than me :)
>
> -
> Lionel
>
> On 04/09/2018 16:27, Roman Gilg wrote:
>
> Ok, I just got a failing assert in xwl_present_flips_stop with the patch when
> opening a context menu in Steam. Seems the xwl_present_flips_stop call is
> coming in too late now after the presenting window has already been changed.
>
[...]
If I read bug 107314 correctly, the crash occurs after the window has
been destroyed, so what about that other patch:
https://patchwork.freedesktop.org/patch/247271/
** plus ** this patch below (just copied for testing purpose), does it
fix th your crash?
From 676597fcd6ee907f4d3f165dd0b5de746f7c8131 Mon Sep 17 00:00:00 2001
From: Olivier Fourdan
Date: Wed, 5 Sep 2018 13:08:03 +0200
Subject: [PATCH xserver] xwayland: ignore sync callback if window is destroyed
If the window is destroyed, there is no need to send the vblank notify
event.
This should avoid a crash in present_vblank_notify()
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107314
Signed-off-by: Olivier Fourdan
---
hw/xwayland/xwayland-present.c | 4
1 file changed, 4 insertions(+)
diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c
index 316e04443..b1751c846 100644
--- a/hw/xwayland/xwayland-present.c
+++ b/hw/xwayland/xwayland-present.c
@@ -276,6 +276,10 @@ xwl_present_sync_callback(void *data,
event->pending = FALSE;
+/* Is the window destroyed already ? */
+if (!xwl_present_window)
+return;
+
if (event->abort) {
/* Event might have been aborted */
if (event->buffer_released)
--
2.17.1
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v3] xwayland: Remove xwl_present_window from privates on cleanup
On 05/09/2018 11:44, Olivier Fourdan wrote: Hi, On Wed, Sep 5, 2018 at 12:05 PM Lionel Landwerlin wrote: On 05/09/2018 09:49, Olivier Fourdan wrote: [...] This is because `xwl_present_cleanup()` frees the memory but does not remove it from the window's privates, and `xwl_present_abort_vblank()` will still find it and hence try to access that freed memory... Remove `xwl_present_window` from window's privates on cleanup so that no other function can find and reuse that data once it's freed. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 We also have this fdo bug : https://bugs.freedesktop.org/show_bug.cgi?id=107314 Yeap, I know about this bug and the patch you sent, but the backtrace is different, it doesn't occur on DestroyWindow(), so I doubt this is the same (unless you tried my fix and it fixes your issue as well...) Cheers, Olivier ___ Oops indeed. ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v3] xwayland: Remove xwl_present_window from privates on cleanup
Hi, On Wed, Sep 5, 2018 at 12:05 PM Lionel Landwerlin wrote: > > On 05/09/2018 09:49, Olivier Fourdan wrote: > > [...] > > This is because `xwl_present_cleanup()` frees the memory but does not > > remove it from the window's privates, and `xwl_present_abort_vblank()` > > will still find it and hence try to access that freed memory... > > > > Remove `xwl_present_window` from window's privates on cleanup so that no > > other function can find and reuse that data once it's freed. > > > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 > > > We also have this fdo bug : > https://bugs.freedesktop.org/show_bug.cgi?id=107314 Yeap, I know about this bug and the patch you sent, but the backtrace is different, it doesn't occur on DestroyWindow(), so I doubt this is the same (unless you tried my fix and it fixes your issue as well...) Cheers, Olivier ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v3] xwayland: Remove xwl_present_window from privates on cleanup
On 05/09/2018 09:49, Olivier Fourdan wrote: Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()` before the common `DestroyWindow()`. But then `DestroyWindow()` calls `present_destroy_window()` which will possibly end up in `xwl_present_abort_vblank()` which will try to access data that was previously freed by `xwl_present_cleanup()`: Invalid read of size 8 at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) by 0x53695A: present_free_window_vblank (present_screen.c:87) by 0x53695A: present_destroy_window (present_screen.c:152) by 0x42A90D: xwl_destroy_window (xwayland.c:653) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) Address 0x182abde0 is 80 bytes inside a block of size 112 free'd at 0x4C2FDAC: free (vg_replace_malloc.c:530) by 0x42A937: xwl_destroy_window (xwayland.c:647) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) by 0x446DA5: ProcKillClient (dispatch.c:3279) by 0x4476AF: Dispatch (dispatch.c:479) Block was alloc'd at at 0x4C30B06: calloc (vg_replace_malloc.c:711) by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) by 0x539728: proc_present_query_capabilities (present_request.c:227) by 0x4476AF: Dispatch (dispatch.c:479) by 0x44B5B5: dix_main (main.c:276) by 0x75F611A: (below main) (libc-start.c:308) This is because `xwl_present_cleanup()` frees the memory but does not remove it from the window's privates, and `xwl_present_abort_vblank()` will still find it and hence try to access that freed memory... Remove `xwl_present_window` from window's privates on cleanup so that no other function can find and reuse that data once it's freed. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 We also have this fdo bug : https://bugs.freedesktop.org/show_bug.cgi?id=107314 Signed-off-by: Olivier Fourdan --- v2: Remove leftovers from broken initial patch... v3: Rephrase non-sensical explanation of the fix hw/xwayland/xwayland-present.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 81e0eb9ce..316e04443 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -147,6 +147,11 @@ xwl_present_cleanup(WindowPtr window) /* Clear timer */ xwl_present_free_timer(xwl_present_window); +/* Remove from privates so we don't try to access it later */ +dixSetPrivate(>devPrivates, + _present_window_private_key, + NULL); + free(xwl_present_window); } ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver v3] xwayland: Remove xwl_present_window from privates on cleanup
Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()` before the common `DestroyWindow()`. But then `DestroyWindow()` calls `present_destroy_window()` which will possibly end up in `xwl_present_abort_vblank()` which will try to access data that was previously freed by `xwl_present_cleanup()`: Invalid read of size 8 at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) by 0x53695A: present_free_window_vblank (present_screen.c:87) by 0x53695A: present_destroy_window (present_screen.c:152) by 0x42A90D: xwl_destroy_window (xwayland.c:653) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) Address 0x182abde0 is 80 bytes inside a block of size 112 free'd at 0x4C2FDAC: free (vg_replace_malloc.c:530) by 0x42A937: xwl_destroy_window (xwayland.c:647) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) by 0x446DA5: ProcKillClient (dispatch.c:3279) by 0x4476AF: Dispatch (dispatch.c:479) Block was alloc'd at at 0x4C30B06: calloc (vg_replace_malloc.c:711) by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) by 0x539728: proc_present_query_capabilities (present_request.c:227) by 0x4476AF: Dispatch (dispatch.c:479) by 0x44B5B5: dix_main (main.c:276) by 0x75F611A: (below main) (libc-start.c:308) This is because `xwl_present_cleanup()` frees the memory but does not remove it from the window's privates, and `xwl_present_abort_vblank()` will still find it and hence try to access that freed memory... Remove `xwl_present_window` from window's privates on cleanup so that no other function can find and reuse that data once it's freed. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 Signed-off-by: Olivier Fourdan --- v2: Remove leftovers from broken initial patch... v3: Rephrase non-sensical explanation of the fix hw/xwayland/xwayland-present.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 81e0eb9ce..316e04443 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -147,6 +147,11 @@ xwl_present_cleanup(WindowPtr window) /* Clear timer */ xwl_present_free_timer(xwl_present_window); +/* Remove from privates so we don't try to access it later */ +dixSetPrivate(>devPrivates, + _present_window_private_key, + NULL); + free(xwl_present_window); } -- 2.17.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver v2] xwayland: Remove xwl_present_window from privates on cleanup
On 2018-09-05 10:35 a.m., Olivier Fourdan wrote: > Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()` > before the common `DestroyWindow()`. > > But then `DestroyWindow()` calls `present_destroy_window()` which will > possibly end up in `xwl_present_abort_vblank()` which will try to access > data that was previously freed by `xwl_present_cleanup()`: > > Invalid read of size 8 > at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) > by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) > by 0x53695A: present_free_window_vblank (present_screen.c:87) > by 0x53695A: present_destroy_window (present_screen.c:152) > by 0x42A90D: xwl_destroy_window (xwayland.c:653) > by 0x584298: compDestroyWindow (compwindow.c:613) > by 0x53CEE3: damageDestroyWindow (damage.c:1570) > by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) > by 0x46F7F6: FreeWindowResources (window.c:1031) > by 0x472847: DeleteWindow (window.c:1099) > by 0x46B54C: doFreeResource (resource.c:880) > by 0x46C706: FreeClientResources (resource.c:1146) > by 0x446ADE: CloseDownClient (dispatch.c:3473) >Address 0x182abde0 is 80 bytes inside a block of size 112 free'd > at 0x4C2FDAC: free (vg_replace_malloc.c:530) > by 0x42A937: xwl_destroy_window (xwayland.c:647) > by 0x584298: compDestroyWindow (compwindow.c:613) > by 0x53CEE3: damageDestroyWindow (damage.c:1570) > by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) > by 0x46F7F6: FreeWindowResources (window.c:1031) > by 0x472847: DeleteWindow (window.c:1099) > by 0x46B54C: doFreeResource (resource.c:880) > by 0x46C706: FreeClientResources (resource.c:1146) > by 0x446ADE: CloseDownClient (dispatch.c:3473) > by 0x446DA5: ProcKillClient (dispatch.c:3279) > by 0x4476AF: Dispatch (dispatch.c:479) >Block was alloc'd at > at 0x4C30B06: calloc (vg_replace_malloc.c:711) > by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) > by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) > by 0x539728: proc_present_query_capabilities (present_request.c:227) > by 0x4476AF: Dispatch (dispatch.c:479) > by 0x44B5B5: dix_main (main.c:276) > by 0x75F611A: (below main) (libc-start.c:308) > > This is because `xwl_present_cleanup()` frees the memory but does not > remove it from the window's privates, and `xwl_present_abort_vblank()` > will still find it and hence try to access that freed memory... > > Clear `xwl_present_cleanup()` after `DestroyWindow()` so that > `xwl_present_abort_vblank()` can still access valid memory before it's > freed. This last paragraph doesn't seem to match the rest of the patch. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver v2] xwayland: Remove xwl_present_window from privates on cleanup
Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()` before the common `DestroyWindow()`. But then `DestroyWindow()` calls `present_destroy_window()` which will possibly end up in `xwl_present_abort_vblank()` which will try to access data that was previously freed by `xwl_present_cleanup()`: Invalid read of size 8 at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) by 0x53695A: present_free_window_vblank (present_screen.c:87) by 0x53695A: present_destroy_window (present_screen.c:152) by 0x42A90D: xwl_destroy_window (xwayland.c:653) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) Address 0x182abde0 is 80 bytes inside a block of size 112 free'd at 0x4C2FDAC: free (vg_replace_malloc.c:530) by 0x42A937: xwl_destroy_window (xwayland.c:647) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) by 0x446DA5: ProcKillClient (dispatch.c:3279) by 0x4476AF: Dispatch (dispatch.c:479) Block was alloc'd at at 0x4C30B06: calloc (vg_replace_malloc.c:711) by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) by 0x539728: proc_present_query_capabilities (present_request.c:227) by 0x4476AF: Dispatch (dispatch.c:479) by 0x44B5B5: dix_main (main.c:276) by 0x75F611A: (below main) (libc-start.c:308) This is because `xwl_present_cleanup()` frees the memory but does not remove it from the window's privates, and `xwl_present_abort_vblank()` will still find it and hence try to access that freed memory... Clear `xwl_present_cleanup()` after `DestroyWindow()` so that `xwl_present_abort_vblank()` can still access valid memory before it's freed. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 Signed-off-by: Olivier Fourdan --- v2: Remove leftovers from broken initial patch... hw/xwayland/xwayland-present.c | 5 + 1 file changed, 5 insertions(+) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 81e0eb9ce..316e04443 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -147,6 +147,11 @@ xwl_present_cleanup(WindowPtr window) /* Clear timer */ xwl_present_free_timer(xwl_present_window); +/* Remove from privates so we don't try to access it later */ +dixSetPrivate(>devPrivates, + _present_window_private_key, + NULL); + free(xwl_present_window); } -- 2.17.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] xwayland: Remove xwl_present_window from privates on cleanup
Xwayland's `xwl_destroy_window()` invokes `xwl_present_cleanup()` before the common `DestroyWindow()`. But then `DestroyWindow()` calls `present_destroy_window()` which will possibly end up in `xwl_present_abort_vblank()` which will try to access data that was previously freed by `xwl_present_cleanup()`: Invalid read of size 8 at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) by 0x53695A: present_free_window_vblank (present_screen.c:87) by 0x53695A: present_destroy_window (present_screen.c:152) by 0x42A90D: xwl_destroy_window (xwayland.c:653) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) Address 0x182abde0 is 80 bytes inside a block of size 112 free'd at 0x4C2FDAC: free (vg_replace_malloc.c:530) by 0x42A937: xwl_destroy_window (xwayland.c:647) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) by 0x446DA5: ProcKillClient (dispatch.c:3279) by 0x4476AF: Dispatch (dispatch.c:479) Block was alloc'd at at 0x4C30B06: calloc (vg_replace_malloc.c:711) by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) by 0x539728: proc_present_query_capabilities (present_request.c:227) by 0x4476AF: Dispatch (dispatch.c:479) by 0x44B5B5: dix_main (main.c:276) by 0x75F611A: (below main) (libc-start.c:308) This is because `xwl_present_cleanup()` frees the memory but does not remove it from the window's privates, and `xwl_present_abort_vblank()` will still find it and hence try to access that freed memory... Clear `xwl_present_cleanup()` after `DestroyWindow()` so that `xwl_present_abort_vblank()` can still access valid memory before it's freed. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 Signed-off-by: Olivier Fourdan --- hw/xwayland/xwayland-present.c | 5 + hw/xwayland/xwayland.c | 10 +- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/hw/xwayland/xwayland-present.c b/hw/xwayland/xwayland-present.c index 81e0eb9ce..316e04443 100644 --- a/hw/xwayland/xwayland-present.c +++ b/hw/xwayland/xwayland-present.c @@ -147,6 +147,11 @@ xwl_present_cleanup(WindowPtr window) /* Clear timer */ xwl_present_free_timer(xwl_present_window); +/* Remove from privates so we don't try to access it later */ +dixSetPrivate(>devPrivates, + _present_window_private_key, + NULL); + free(xwl_present_window); } diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c index 96b4db18c..63d69fb3a 100644 --- a/hw/xwayland/xwayland.c +++ b/hw/xwayland/xwayland.c @@ -642,11 +642,6 @@ xwl_destroy_window(WindowPtr window) struct xwl_screen *xwl_screen = xwl_screen_get(screen); Bool ret; -#ifdef GLAMOR_HAS_GBM -if (xwl_screen->present) -xwl_present_cleanup(window); -#endif - screen->DestroyWindow = xwl_screen->DestroyWindow; if (screen->DestroyWindow) @@ -657,6 +652,11 @@ xwl_destroy_window(WindowPtr window) xwl_screen->DestroyWindow = screen->DestroyWindow; screen->DestroyWindow = xwl_destroy_window; +#ifdef GLAMOR_HAS_GBM +if (xwl_screen->present) +xwl_present_cleanup(window); +#endif + return ret; } -- 2.17.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] xwayland: Clean-up present after destroying common bits
On Wed, Sep 5, 2018 at 9:48 AM Olivier Fourdan wrote: > > Right now, `xwl_destroy_window()` invokes `xwl_present_cleanup()` before > the common `DestroyWindow()`. > > But `DestroyWindow()` calls in `present_destroy_window()` which will > then end up in `xwl_present_abort_vblank()` which will try to access > data that was previously freed by `xwl_present_cleanup()`: > > Invalid read of size 8 > at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) > by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) > by 0x53695A: present_free_window_vblank (present_screen.c:87) > by 0x53695A: present_destroy_window (present_screen.c:152) > by 0x42A90D: xwl_destroy_window (xwayland.c:653) > by 0x584298: compDestroyWindow (compwindow.c:613) > by 0x53CEE3: damageDestroyWindow (damage.c:1570) > by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) > by 0x46F7F6: FreeWindowResources (window.c:1031) > by 0x472847: DeleteWindow (window.c:1099) > by 0x46B54C: doFreeResource (resource.c:880) > by 0x46C706: FreeClientResources (resource.c:1146) > by 0x446ADE: CloseDownClient (dispatch.c:3473) >Address 0x182abde0 is 80 bytes inside a block of size 112 free'd > at 0x4C2FDAC: free (vg_replace_malloc.c:530) > by 0x42A937: xwl_destroy_window (xwayland.c:647) > by 0x584298: compDestroyWindow (compwindow.c:613) > by 0x53CEE3: damageDestroyWindow (damage.c:1570) > by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) > by 0x46F7F6: FreeWindowResources (window.c:1031) > by 0x472847: DeleteWindow (window.c:1099) > by 0x46B54C: doFreeResource (resource.c:880) > by 0x46C706: FreeClientResources (resource.c:1146) > by 0x446ADE: CloseDownClient (dispatch.c:3473) > by 0x446DA5: ProcKillClient (dispatch.c:3279) > by 0x4476AF: Dispatch (dispatch.c:479) >Block was alloc'd at > at 0x4C30B06: calloc (vg_replace_malloc.c:711) > by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) > by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) > by 0x539728: proc_present_query_capabilities (present_request.c:227) > by 0x4476AF: Dispatch (dispatch.c:479) > by 0x44B5B5: dix_main (main.c:276) > by 0x75F611A: (below main) (libc-start.c:308) > > Move `xwl_present_cleanup()` after `DestroyWindow()` so that > `xwl_present_abort_vblank()` can still access valid memory before it's > freed. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 > Signed-off-by: Olivier Fourdan > --- > hw/xwayland/xwayland.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c > index 96b4db18c..63d69fb3a 100644 > --- a/hw/xwayland/xwayland.c > +++ b/hw/xwayland/xwayland.c > @@ -642,11 +642,6 @@ xwl_destroy_window(WindowPtr window) > struct xwl_screen *xwl_screen = xwl_screen_get(screen); > Bool ret; > > -#ifdef GLAMOR_HAS_GBM > -if (xwl_screen->present) > -xwl_present_cleanup(window); > -#endif > - > screen->DestroyWindow = xwl_screen->DestroyWindow; > > if (screen->DestroyWindow) > @@ -657,6 +652,11 @@ xwl_destroy_window(WindowPtr window) > xwl_screen->DestroyWindow = screen->DestroyWindow; > screen->DestroyWindow = xwl_destroy_window; > > +#ifdef GLAMOR_HAS_GBM > +if (xwl_screen->present) > +xwl_present_cleanup(window); > +#endif > + > return ret; > } > > -- > 2.17.1 > Sorry, not enough coffee, I take that back, obviously the crash won't occur because `xwl_present_cleanup()` will fail to match the window once the resources have been freed so we return early in `xwl_present_cleanup()`... Will try again. Cheers, Olivier ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
Re: [PATCH xserver] glamor: Add support for exporting depth 16 pixmaps.
On 2018-09-05 6:00 a.m., Eric Anholt wrote:
> With a patch to mesa to expose rgb565 pbuffers even on a server with
> only depth 24 and 32 visuals, fixes
> dEQP-EGL.functional.render.single_context.gles2.rgb565_pbuffer. Those
> pbuffers (or at least something renderable with 565) are required by
> the current CTS for GLES3, and having the server support DRI3 on those
> pixmaps means that we can avoid having a different path for EGL
> pbuffers compared to pixmaps.
>
> Signed-off-by: Eric Anholt
> ---
> glamor/glamor_egl.c | 22 ++
> 1 file changed, 14 insertions(+), 8 deletions(-)
>
> diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c
> index b33d8ef1598e..df278b1a1a02 100644
> --- a/glamor/glamor_egl.c
> +++ b/glamor/glamor_egl.c
> @@ -280,18 +280,24 @@ glamor_make_pixmap_exportable(PixmapPtr pixmap, Bool
> modifiers_ok)
> (modifiers_ok || !pixmap_priv->used_modifiers))
> return TRUE;
>
> -if (pixmap->drawable.bitsPerPixel != 32) {
> +switch (pixmap->drawable.depth) {
> +case 30:
> +format = GBM_FORMAT_ARGB2101010;
> +break;
> +case 32:
> +case 24:
> +format = GBM_FORMAT_ARGB;
> +break;
> +case 16:
> +format = GBM_FORMAT_RGB565;
> +break;
> +default:
> xf86DrvMsg(scrn->scrnIndex, X_ERROR,
> - "Failed to make %dbpp pixmap exportable\n",
> - pixmap->drawable.bitsPerPixel);
> + "Failed to make %d depth, %dbpp pixmap exportable\n",
> + pixmap->drawable.depth, pixmap->drawable.bitsPerPixel);
> return FALSE;
> }
>
> -if (pixmap->drawable.depth == 30)
> - format = GBM_FORMAT_ARGB2101010;
> -else
> -format = GBM_FORMAT_ARGB;
> -
> #ifdef GBM_BO_WITH_MODIFIERS
> if (modifiers_ok && glamor_egl->dmabuf_capable) {
> uint32_t num_modifiers;
>
Reviewed-by: Michel Dänzer
--
Earthling Michel Dänzer | http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
[PATCH xserver] xwayland: Clean-up present after destroying common bits
Right now, `xwl_destroy_window()` invokes `xwl_present_cleanup()` before the common `DestroyWindow()`. But `DestroyWindow()` calls in `present_destroy_window()` which will then end up in `xwl_present_abort_vblank()` which will try to access data that was previously freed by `xwl_present_cleanup()`: Invalid read of size 8 at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) by 0x53695A: present_free_window_vblank (present_screen.c:87) by 0x53695A: present_destroy_window (present_screen.c:152) by 0x42A90D: xwl_destroy_window (xwayland.c:653) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) Address 0x182abde0 is 80 bytes inside a block of size 112 free'd at 0x4C2FDAC: free (vg_replace_malloc.c:530) by 0x42A937: xwl_destroy_window (xwayland.c:647) by 0x584298: compDestroyWindow (compwindow.c:613) by 0x53CEE3: damageDestroyWindow (damage.c:1570) by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) by 0x46F7F6: FreeWindowResources (window.c:1031) by 0x472847: DeleteWindow (window.c:1099) by 0x46B54C: doFreeResource (resource.c:880) by 0x46C706: FreeClientResources (resource.c:1146) by 0x446ADE: CloseDownClient (dispatch.c:3473) by 0x446DA5: ProcKillClient (dispatch.c:3279) by 0x4476AF: Dispatch (dispatch.c:479) Block was alloc'd at at 0x4C30B06: calloc (vg_replace_malloc.c:711) by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) by 0x539728: proc_present_query_capabilities (present_request.c:227) by 0x4476AF: Dispatch (dispatch.c:479) by 0x44B5B5: dix_main (main.c:276) by 0x75F611A: (below main) (libc-start.c:308) Move `xwl_present_cleanup()` after `DestroyWindow()` so that `xwl_present_abort_vblank()` can still access valid memory before it's freed. Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 Signed-off-by: Olivier Fourdan --- hw/xwayland/xwayland.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c index 96b4db18c..63d69fb3a 100644 --- a/hw/xwayland/xwayland.c +++ b/hw/xwayland/xwayland.c @@ -642,11 +642,6 @@ xwl_destroy_window(WindowPtr window) struct xwl_screen *xwl_screen = xwl_screen_get(screen); Bool ret; -#ifdef GLAMOR_HAS_GBM -if (xwl_screen->present) -xwl_present_cleanup(window); -#endif - screen->DestroyWindow = xwl_screen->DestroyWindow; if (screen->DestroyWindow) @@ -657,6 +652,11 @@ xwl_destroy_window(WindowPtr window) xwl_screen->DestroyWindow = screen->DestroyWindow; screen->DestroyWindow = xwl_destroy_window; +#ifdef GLAMOR_HAS_GBM +if (xwl_screen->present) +xwl_present_cleanup(window); +#endif + return ret; } -- 2.17.1 ___ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel
