[PATCH weston] RDP compositor: make the seat dynamic and don't destroy it on removal

[Hardening]

Le 25/09/2015 11:31, Pekka Paalanen a écrit :
> On Thu, 24 Sep 2015 23:40:26 +0200
> David FORT <rdp.effort at gmail.com> wrote:
> 
>> This patch makes the seat dynamic and leak it on purpose during seat removal to
>> prevent the ghost object case.
>> ---
>>  src/compositor-rdp.c | 39 +++++++++++++++++++++++++++------------
>>  1 file changed, 27 insertions(+), 12 deletions(-)
> 
> Hi David,
> 
> this patch is still missing the whole explanation of what is going on in
> here. My questions from
> http://lists.freedesktop.org/archives/wayland-devel/2015-May/022055.html
> are still unanswered, and today I understand even less than then. :-)
> 
> Or is there already a comment in the code explaining why rdp-backend
> does funny stuff with seats? I couldn't find it on a quick look.
> 
> Why leak it?
> What is the ghost object problem?
> Why you must use only part of weston_seat_release()?
> 
> I have some very vague memories of wl_seat missing destructor protocol
> or something, is this related?


Hello Pekka,
you're right I've not elaborated much.

So the general problem was that wl_seat doesn't have a release request,
so we can't track the usage of a wl_seat by wayland clients. And as we
can't track the usage, we can't release it safely or we take the risk
that clients could address a released object. In the current situation
we can't safely release a seat object.

So in my patch I'm mallocating a seat (not having it static with the RDP
context), an I took the parts of weston_seat_release() that only do
things internally to weston (so that I release as much as I can).


Perhaps I'm not cleaning up correctly because with my patch applied, if I:
* open a terminal;
* connect with 2 RDP clients (so 2 seats);
* take the focus in the terminal;
* stop weston with a Ctrl-C

I get the following valgrind traceback during weston's shutdown, related
to the input method:
^C[14:35:47.687] caught signal 2
==21909== Invalid read of size 8
==21909==    at 0x4173B9: unbind_input_method (text-backend.c:834)
==21909==    by 0x4E3D37B: destroy_resource (wayland-server.c:537)
==21909==    by 0x4E42255: for_each_helper.isra.0 (wayland-util.c:359)
==21909==    by 0x4E4279E: wl_map_for_each (wayland-util.c:365)
==21909==    by 0x4E3DF57: wl_client_destroy (wayland-server.c:675)
==21909==    by 0x418401: text_backend_destroy (text-backend.c:1046)
==21909==    by 0x93542F2: shell_destroy (shell.c:6473)
==21909==    by 0x4109C6: weston_compositor_destroy
(wayland-server-core.h:264)
==21909==    by 0x40897D: main (main.c:829)
==21909==  Address 0x9b9e6f0 is 112 bytes inside a block of size 120 free'd
==21909==    at 0x4C2BDEC: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==21909==    by 0x6A468AE: rdp_peer_context_free (wayland-server-core.h:264)
==21909==    by 0x6D520DA: freerdp_peer_context_free (peer.c:738)
==21909==    by 0x6A469B7: rdp_client_activity (compositor-rdp.c:674)
==21909==    by 0x4E3FE51: wl_event_loop_dispatch (event-loop.c:422)
==21909==    by 0x4E3E6B4: wl_display_run (wayland-server.c:1004)
==21909==    by 0x408D35: main (main.c:818)
==21909==


Anyway without the patch, valgrind complains during seat releasing (for
example when a RDP peer disconnects).

Regards.
-- 
David FORT
website: http://www.hardening-consulting.com/