Wayland: memory corruption when sending clipboard data

[mahkoh]

mahkoh/jay#1123

I added debug logging in SDL_SetClipboardText and there the text is correct. But in SDL_ClipboardTextCallback the text is corrupted so it was likely freed some time in between.

[Kontrabant]

What version of SDL is this happening with? Testing SDL_SetClipboardText() on the main branch, I'm not seeing corrupted data in SDL_ClipboardTextCallback(), and the text I enter into it is pasted into other applications correctly. Running under Valgrind isn't reporting any issues either.

[mahkoh]

Latest master. I'm not seeing anything with valgrind either. If you tell me how to compile SDL with address sanitizer, I can try that.

[Kontrabant]

Just add -DSDL_ASAN=ON to the cmake configuration options.

[mahkoh]

That didn't yield any results but I got it to work with valgrind:

==2843746== Warning: unimplemented fcntl command: 1027
==2843746== Invalid read of size 1
==2843746==    at 0x4858E66: strlen (vg_replace_strmem.c:506)
==2843746==    by 0x49C2816: SDL_ClipboardTextCallback (SDL_clipboard.c:347)
==2843746==    by 0x4A535B9: Wayland_data_source_send (SDL_waylanddatamanager.c:282)
==2843746==    by 0x5544CD5: ffi_call_unix64 (unix64.S:105)
==2843746==    by 0x55409AC: ffi_call_int.lto_priv.0 (ffi64.c:734)
==2843746==    by 0x5543D51: ffi_call (ffi64.c:1113)
==2843746==    by 0x553077C: wl_closure_invoke.constprop.0 (connection.c:1243)
==2843746==    by 0x5531608: dispatch_event (wayland-client.c:1731)
==2843746==    by 0x5531A12: UnknownInlinedFun (wayland-client.c:1877)
==2843746==    by 0x5531A12: wl_display_dispatch_queue_pending (wayland-client.c:2242)
==2843746==    by 0x4A5DD6B: Wayland_PumpEvents (SDL_waylandevents.c:586)
==2843746==    by 0x48EFE6C: SDL_PumpEventsInternal (SDL_events.c:1552)
==2843746==    by 0x4920724: SDL_IterateMainCallbacks (SDL_main_callbacks.c:124)
==2843746==  Address 0x1d9803d0 is 0 bytes inside a block of size 34 free'd
==2843746==    at 0x48529EF: free (vg_replace_malloc.c:989)
==2843746==    by 0x49C2924: SDL_CancelClipboardData (SDL_clipboard.c:55)
==2843746==    by 0x48EB706: SDL_SendClipboardUpdate (SDL_clipboardevents.c:32)
==2843746==    by 0x5544CD5: ffi_call_unix64 (unix64.S:105)
==2843746==    by 0x55409AC: ffi_call_int.lto_priv.0 (ffi64.c:734)
==2843746==    by 0x5543D51: ffi_call (ffi64.c:1113)
==2843746==    by 0x553077C: wl_closure_invoke.constprop.0 (connection.c:1243)
==2843746==    by 0x5531608: dispatch_event (wayland-client.c:1731)
==2843746==    by 0x5531A12: UnknownInlinedFun (wayland-client.c:1877)
==2843746==    by 0x5531A12: wl_display_dispatch_queue_pending (wayland-client.c:2242)
==2843746==    by 0x4A5DD6B: Wayland_PumpEvents (SDL_waylandevents.c:586)
==2843746==    by 0x48EFE6C: SDL_PumpEventsInternal (SDL_events.c:1552)
==2843746==    by 0x4920724: SDL_IterateMainCallbacks (SDL_main_callbacks.c:124)
==2843746==  Block was alloc'd at
==2843746==    at 0x484F8A8: malloc (vg_replace_malloc.c:446)
==2843746==    by 0x497B6E7: real_malloc (SDL_malloc.c:6452)
==2843746==    by 0x497B6E7: SDL_malloc_REAL (SDL_malloc.c:6570)
==2843746==    by 0x497E209: SDL_strdup_REAL (SDL_string.c:1115)
==2843746==    by 0x49C346E: SDL_SetClipboardText_REAL (SDL_clipboard.c:367)
==2843746==    by 0x40016EC: SDL_AppEvent (clipboard.c:98)
==2843746==    by 0x492053B: SDL_DispatchMainCallbackEvent (SDL_main_callbacks.c:50)
==2843746==    by 0x492053B: SDL_DispatchMainCallbackEvent (SDL_main_callbacks.c:47)
==2843746==    by 0x492053B: SDL_DispatchMainCallbackEvents (SDL_main_callbacks.c:66)
==2843746==    by 0x49206DD: SDL_IterateMainCallbacks (SDL_main_callbacks.c:126)
==2843746==    by 0x4AEF3FE: GenericIterateMainCallbacks (SDL_sysmain_callbacks.c:65)
==2843746==    by 0x4AEF3FE: SDL_EnterAppMainCallbacks_REAL (SDL_sysmain_callbacks.c:76)
==2843746==    by 0x4C3B740: (below main) (libc_start_call_main.h:59)

[Kontrabant]

Can you grab a WAYLAND_DEBUG=1 log as well?

[mahkoh]

logs.gz

[Kontrabant]

Hmm, between the Valgrind and Wayland debug logs, here's what I think is happening:

• The clipboard text is set and a data offer is created
• The data offer is recursively advertised back to SDL
• SDL checks the application/x-sdl3-source-id MIME type to see if this offer originated from itself
• For some reason, the check fails and SDL thinks the offer is external, so the internal clipboard data is cleared
• A subsequent attempt to read the clipboard text tries to read the old, cleared data, since the offer isn't actually external, resulting in reading a freed string

The code can be more robust against failure in this case, but the question is why reading application/x-sdl3-source-id indicates that the offer is external, when it isn't?

[mahkoh]

• The data offer is recursively advertised back to SDL
• SDL checks the application/x-sdl3-source-id MIME type to see if this offer originated from itself
• For some reason, the check fails and SDL thinks the offer is external, so the internal clipboard data is cleared

I don't believe that is correct behavior. Let X be any other application.

• X sets the selection
• SDL sets the selection
• SDL receives the offer for X's selection
• SDL destroys its own selection according to what you wrote
• SDL receives the offer for its own selection but it's too late now

SDL needs to keep its own latest selection alive independently of which offers it receives. It cannot know if its selection might still receive send events in the future.

[mahkoh]

If you do want to follow this model, then maybe doing a roundtrip every time you set the selection might work and then only act on the latest offer you received after the roundtrip completes.

[Kontrabant]

At the Wayland level, selections are kept alive until the data source is cancelled. Incoming selection offers clear the data in the platform-independent middle-layer.

SDL's clipboard model is designed such that setting and then querying the clipboard data internally should never reach the platform layer to begin with. At the Wayland level, SDL tags its source selections with a unique identifier, and ignores recursive selection offers, since internal requests from clients should be handled entirely within the platform-independent clipboard system, without a round-trip through the platform layer.

The core problem here, is that the incoming selection offer is not being detected as recursive, when it should be.

[mahkoh]

But in the example I described above the first incoming offer after SDL sets the selection is not from SDL. The second one is. Are you sure that's not what's happening here?

[Kontrabant]

From the debug log, the incoming offers have a MIME type of application/x-sdl3-source-id which indicates that they're originating from SDL. SDL then requests a receive for the application/x-sdl3-source-id data, the data is written to the pipe, SDL reads it back, and checks if it matches the internal ID. If it does, the incoming offer is recursive, and ignored. In this case, however, the following from the Valgrind logs occurs:

==2843746==    by 0x49C2924: SDL_CancelClipboardData (SDL_clipboard.c:55)
==2843746==    by 0x48EB706: SDL_SendClipboardUpdate (SDL_clipboardevents.c:32)

The only way these functions would be called in that sequence is when the data from application/x-sdl3-source-id doesn't match the internal ID, which it should it in this case as in both cases where the source ID is checked, each receive request has a matching send in the logs, so SDL is just sending data back to itself.