| Summary: | Crash on resuming laptop with screen connected | ||
|---|---|---|---|
| Product: | Wayland | Reporter: | Lionel Landwerlin <lionel.g.landwerlin> |
| Component: | XWayland | Assignee: | Wayland bug list <wayland-bugs> |
| Status: | RESOLVED MOVED | QA Contact: | Xorg Project Team <xorg-team> |
| Severity: | normal | ||
| Priority: | medium | CC: | fourdan |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
present: fix freed pointer access
[RFC PATCH xserver] xwayland: ignore sync callback if window is destroyed |
||
|
Description
Lionel Landwerlin
2018-07-20 19:40:23 UTC
This is XWayland 1.20.0 *** Bug 107427 has been marked as a duplicate of this bug. *** Reproduced with xwayland 1.20.1. Got a valgrind trace : ==5331== Invalid read of size 8 ==5331== at 0x212B4D: present_vblank_notify (present_vblank.c:34) ==5331== by 0x21439B: present_wnmd_flip_notify (present_wnmd.c:194) ==5331== by 0x21439B: present_wnmd_event_notify (present_wnmd.c:228) ==5331== by 0x156216: xwl_present_sync_callback (xwayland-present.c:282) ==5331== by 0x6570FCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4) ==5331== by 0x657093E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4) ==5331== by 0x4DDB183: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0) ==5331== by 0x4DD79D8: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0) ==5331== by 0x4DD8EA3: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0) ==5331== by 0x14BCCA: xwl_read_events (xwayland.c:814) ==5331== by 0x2AC0D0: ospoll_wait (ospoll.c:651) ==5331== by 0x2A5322: WaitForSomething (WaitFor.c:208) ==5331== by 0x27574B: Dispatch (dispatch.c:421) ==5331== Address 0x1b44dc98 is 40 bytes inside a block of size 184 free'd ==5331== at 0x48369EB: free (vg_replace_malloc.c:530) ==5331== by 0x213B0A: present_wnmd_free_idle_vblanks (present_wnmd.c:118) ==5331== by 0x213B0A: present_wnmd_flips_stop (present_wnmd.c:161) ==5331== by 0x2143EF: present_wnmd_flip_notify (present_wnmd.c:192) ==5331== by 0x2143EF: present_wnmd_event_notify (present_wnmd.c:228) ==5331== by 0x156216: xwl_present_sync_callback (xwayland-present.c:282) ==5331== by 0x6570FCD: ffi_call_unix64 (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4) ==5331== by 0x657093E: ffi_call (in /usr/lib/x86_64-linux-gnu/libffi.so.6.0.4) ==5331== by 0x4DDB183: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0) ==5331== by 0x4DD79D8: ??? (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0) ==5331== by 0x4DD8EA3: wl_display_dispatch_queue_pending (in /usr/lib/x86_64-linux-gnu/libwayland-client.so.0.3.0) ==5331== by 0x14BCCA: xwl_read_events (xwayland.c:814) ==5331== by 0x2AC0D0: ospoll_wait (ospoll.c:651) ==5331== by 0x2A5322: WaitForSomething (WaitFor.c:208) ==5331== Block was alloc'd at ==5331== at 0x48377D5: calloc (vg_replace_malloc.c:711) ==5331== by 0x212D9F: present_vblank_create (present_vblank.c:69) ==5331== by 0x214014: present_wnmd_pixmap (present_wnmd.c:610) ==5331== by 0x21576C: proc_present_pixmap (present_request.c:150) ==5331== by 0x27599D: Dispatch (dispatch.c:479) ==5331== by 0x279945: dix_main (main.c:276) ==5331== by 0x633AB16: (below main) (libc-start.c:310) ==5331== Seems to indicate that we're not dealing with the abort flip correctly. present_wnmd_flip_notify needs fixing. Created attachment 141273 [details] [review] present: fix freed pointer access Tentative fix. Seems safe enough. When we free the vblanks (which, when we're aborting here, can only happen because the window has been destroyed), we'll send a Present IdleNotify. I don't think there's any need to send any completion events. Can you please git-send-email this to xorg-devel@lists.x.org with this on it? Reviewed-by: Daniel Stone <daniels@collabora.com> Created attachment 141463 [details] [review] [RFC PATCH xserver] xwayland: ignore sync callback if window is destroyed (In reply to Daniel Stone from comment #6) > Seems safe enough. When we free the vblanks (which, when we're aborting > here, can only happen because the window has been destroyed), [...] Roman pointed out on the ML a crash possibly caused by that patch. What about this patch then? On destroy, `xwl_present_cleanup()` would free the `xwl_present_window` but leave its reference in the window's privates, so that other functions could still find it and use freed memory. That patch there https://patchwork.freedesktop.org/patch/247271/ fixes that, so with this if we just check for `xwl_present_window` being nun-null in `xwl_present_sync_callback()` we should avoid the crash, cleanly, no? If the idea makes sense (and if someone cwould be willing to try it) and if it works, I would send that to the ML for further review. -- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/724. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.