After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770387 - [Wayland] mutter crashes with fatal error "Could not import pending buffer, ignoring commit"
[Wayland] mutter crashes with fatal error "Could not import pending buffer, i...
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
git master
Other Linux
: Normal critical
: ---
Assigned To: mutter-maint
mutter-maint
: 770140 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2016-08-25 12:43 UTC by Olivier Fourdan
Modified: 2016-10-10 13:02 UTC
See Also:
GNOME target: 3.22
GNOME version: ---


Attachments
bt full (8.83 KB, text/plain)
2016-08-25 12:43 UTC, Olivier Fourdan
  Details
[PATCH] wayland: Survive an unsupported buffer size (2.45 KB, patch)
2016-08-26 07:47 UTC, Olivier Fourdan
none Details | Review
[PATCH] wayland: Survive an unsupported buffer size (2.46 KB, patch)
2016-08-26 07:49 UTC, Olivier Fourdan
committed Details | Review

Description Olivier Fourdan 2016-08-25 12:43:28 UTC
Created attachment 334137 [details]
bt full

Summary:

Typing a string in gedit's search field in the open file popup, mutter/gnome-shell crashes with:

    Could not import pending buffer, ignoring commit

How reproducible

100%

Steps to reproduce

1. Open gedit
2. Click on "Open"
3. Start yping in the search field

Actual result:

gnome-shell/mutter dies taking the whole session with it.

Expected results:

No crash

Additional data:

(gdb) bt
  • #0 meta_fatal
    at core/util.c line 468
  • #1 meta_wayland_buffer_ensure_texture
    at wayland/meta-wayland-buffer.c line 110
  • #2 apply_pending_state
    at wayland/meta-wayland-surface.c line 737
  • #3 ffi_call_unix64
    at ../src/x86/unix64.S line 76
  • #4 ffi_call
    at ../src/x86/ffi64.c line 525
  • #5 wl_closure_invoke
    at src/connection.c line 935
  • #6 wl_client_connection_data
    at src/wayland-server.c line 371
  • #7 wl_event_loop_dispatch
    at src/event-loop.c line 423
  • #8 wayland_event_source_dispatch
    at wayland/meta-wayland.c line 79
  • #9 g_main_dispatch
    at gmain.c line 3201
  • #10 g_main_context_dispatch
    at gmain.c line 3854
  • #11 g_main_context_iterate
    at gmain.c line 3927
  • #12 g_main_loop_run
    at gmain.c line 4123
  • #13 meta_run
    at core/main.c line 572
  • #14 main
    at core/mutter.c line 85

Comment 1 Olivier Fourdan 2016-08-25 13:32:01 UTC
The Cogl error from cogl_wayland_texture_2d_new_from_buffer() is "Failed to create texture 2d due to size/format constraints"
Comment 2 Olivier Fourdan 2016-08-25 14:12:06 UTC
And cogl fails because the given buffer has an invalid height (448x19896)
Comment 3 Olivier Fourdan 2016-08-25 14:35:56 UTC
WAYLAND_DEBUG to the rescue....

What we see is that cogl_wayland_texture_2d_new_from_buffer() fails because of size/format contriants, the buffer size looks very suspicious, it grows like:

  Cogl-Message: shm buffer (448x251)
  Cogl-Message: shm buffer (448x611)
  Cogl-Message: shm buffer (448x22176)

and then cogl_wayland_texture_2d_new_from_buffer() fails

On the other hand, running gedit with WAYLAND_DEBUG shows that the size is indeed set from gtk+

[892129.655]  -> wl_shm@5.create_pool(new id wl_shm_pool@45, fd 17, 39739392)
[892129.693]  -> wl_shm_pool@45.create_buffer(new id wl_buffer@50, 0, 448, 22176, 1792, 0)
[892288.393]  -> wl_surface@33.attach(wl_buffer@50, 0, 0)
[892288.432]  -> wl_surface@33.set_buffer_scale(1)
[892288.436]  -> wl_surface@33.damage(0, 0, 448, 22176)
[892288.457]  -> wl_surface@33.frame(new id wl_callback@49)
[892288.464]  -> wl_surface@33.commit()
[...]

So the root cause might be in gtk+, but it's somehow worrisome that a client bug can take the entire compositor down.

Besides, even if 448x22176 might seem large, it's still way smaller than the limit of 65535 in _gdk_wayland_display_create_window_impl()
Comment 4 Olivier Fourdan 2016-08-25 14:54:00 UTC
The error is raised when _cogl_texture_2d_gl_can_create() returns false, i.e. when the driver cannot support a texture of the given size.
Comment 5 Jonas Ådahl 2016-08-25 16:07:50 UTC
Seems like this is a client side bug then. Then again, mutter should handle misbehaving clients, maybe send an OOM if the buffer is too large?
Comment 6 Jonas Ådahl 2016-08-25 16:08:26 UTC
FWIW, I can't seem to reproduce this issue. I tried the steps described in the first comment, but it works fine.
Comment 7 Olivier Fourdan 2016-08-25 16:12:08 UTC
(In reply to Jonas Ådahl from comment #5)
> Seems like this is a client side bug then. Then again, mutter should handle
> misbehaving clients, maybe send an OOM if the buffer is too large?

Yeah, I'm working on such a patch to post an error to a client when we can't deal with the given buffer

(In reply to Jonas Ådahl from comment #6)
> FWIW, I can't seem to reproduce this issue. I tried the steps described in
> the first comment, but it works fine.

As it depends on the driver used, I guess your HW/driver can support larger textures than mine...
Comment 8 Olivier Fourdan 2016-08-26 07:47:04 UTC
Created attachment 334205 [details] [review]
[PATCH] wayland: Survive an unsupported buffer size

If cogl could create a texture from the client's given buffer, mutter
would raise a fatal error and quit.

As a result, a broken client might kill gnome-shell/mutter and take the
entire Wayland session with it.

Instead of raising a fatal error in this case, log the cogl error
message and send the client an OOM error, so mutter/gnome-shell can
survive an unsupported buffer size.
Comment 9 Olivier Fourdan 2016-08-26 07:49:00 UTC
Created attachment 334206 [details] [review]
[PATCH] wayland: Survive an unsupported buffer size

Fix the nonsensical commit message...
Comment 10 Olivier Fourdan 2016-08-26 08:33:40 UTC
Note, the root cause on gtk+/gedit side is being investigated in bug 770430
Comment 11 Jonas Ådahl 2016-08-26 08:51:33 UTC
Review of attachment 334206 [details] [review]:

This looks good for now. I suppose it'd be better to make state applying handle errors at all, so that we don't continue applying state just as before, possibly breaking various assumptions. But lets leave that for another patch, and stop crashing for now.
Comment 12 Olivier Fourdan 2016-08-26 09:17:35 UTC
Comment on attachment 334206 [details] [review]
[PATCH] wayland: Survive an unsupported buffer size

(In reply to Jonas Ådahl from comment #11)
> Review of attachment 334206 [details] [review] [review]:
> 
> This looks good for now. I suppose it'd be better to make state applying
> handle errors at all, so that we don't continue applying state just as
> before, possibly breaking various assumptions. But lets leave that for
> another patch, and stop crashing for now.

Agreed.

Meanwhile, attachment 334206 [details] [review] pushed in branch master as commit 1f570d5 - wayland: Survive an unsupported buffer size
Comment 13 Yanko Kaneti 2016-10-10 13:02:59 UTC
*** Bug 770140 has been marked as a duplicate of this bug. ***