Bug 93833 - NULL dereference in weston_pointer_send_frame with RDP backend
Summary: NULL dereference in weston_pointer_send_frame with RDP backend
Status: RESOLVED WORKSFORME
Alias: None
Product: Wayland
Classification: Unclassified
Component: weston (show other bugs)
Version: unspecified
Hardware: x86-64 (AMD64) Linux (All)
: high major
Assignee: Wayland bug list
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-23 19:31 UTC by Laurentiu Nicola
Modified: 2016-07-05 13:45 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Fix NULL dereference (630 bytes, patch)
2016-01-26 06:08 UTC, Laurentiu Nicola
Details | Splinter Review

Description Laurentiu Nicola 2016-01-23 19:31:44 UTC
I'm getting this crash when connecting with the Windows RDP client (mstsc).

#0  weston_pointer_send_frame (pointer=0xec7080) at ../src/input.c:394
#1  0x0000000000411031 in default_grab_pointer_frame (grab=<optimized out>) at ../src/input.c:416
#2  0x0000000000413410 in notify_pointer_frame (seat=seat@entry=0x71c628) at ../src/input.c:1370
#3  0x00007ffff61c4f27 in xf_mouseEvent (input=<optimized out>, flags=<optimized out>, x=<optimized out>, y=<optimized out>) at ../src/compositor-rdp.c:1001
#4  0x00007ffff5f0f61c in fastpath_recv_inputs () from /usr/lib/libfreerdp.so.2.0
#5  0x00007ffff5f1f1b7 in ?? () from /usr/lib/libfreerdp.so.2.0
#6  0x00007ffff5f1f2e8 in ?? () from /usr/lib/libfreerdp.so.2.0
#7  0x00007ffff5f124f0 in transport_check_fds () from /usr/lib/libfreerdp.so.2.0
#8  0x00007ffff5f0b338 in rdp_check_fds () from /usr/lib/libfreerdp.so.2.0
#9  0x00007ffff5f1ef23 in ?? () from /usr/lib/libfreerdp.so.2.0
#10 0x00007ffff61c4c88 in rdp_client_activity (fd=<optimized out>, mask=<optimized out>, data=0x71dab0) at ../src/compositor-rdp.c:658
#11 0x00007ffff7bd2462 in wl_event_loop_dispatch () from /usr/lib/libwayland-server.so.0
#12 0x00007ffff7bd0cc5 in wl_display_run () from /usr/lib/libwayland-server.so.0
#13 0x000000000041c25b in main (argc=1, argv=0x7fffffffe4f8) at ../src/main.c:859

(gdb) p pointer->focus_client
$3 = (struct weston_pointer_client *) 0x0

I'm using commit 1abf5e43b56fca493888f43e3261126b320e5cbb.
Comment 1 Laurentiu Nicola 2016-01-23 19:39:19 UTC
Adding a NULL check seems to yield:

[21:38:31:738] [12806:12806] [INFO][com.freerdp.core.gcc] - Active rdp encryption level: NONE
[21:38:31:738] [12806:12806] [INFO][com.freerdp.core.gcc] - Selected rdp encryption method: NONE
[21:38:31.442] kbd_layout:0x409 kbd_type:0x7 kbd_subType:0x0 kbd_functionKeys:0xc
[21:38:31.442] xf_peer_activate: matching layout=us variant=(null)
[21:38:31.537] unable to checkDescriptor for 0x6edda0
[Thread 0x7fffe7f4d700 (LWP 12818) exited]
[Thread 0x7fffe874e700 (LWP 12819) exited]
[Thread 0x7fffe774c700 (LWP 12817) exited]
[Thread 0x7fffe6f4b700 (LWP 12816) exited]
[21:38:31.540] input_method disconnected, respawning...
[21:38:31.540] launching '/usr/lib/weston/weston-keyboard'
wl_registry@2: error 0: invalid global wl_seat (14)
[21:38:31.543] Error: /usr/lib/weston/weston-desktop-shell apparently cannot run at all.
               Quitting...wl_registry@2: error 0: invalid global wl_seat (14)
Comment 2 Laurentiu Nicola 2016-01-23 19:51:12 UTC
Connecting with freerdp seems to work, at least with my patch applied.
Comment 3 Jonas Ådahl 2016-01-26 01:13:12 UTC
(In reply to Laurentiu Nicola from comment #2)
> Connecting with freerdp seems to work, at least with my patch applied.

Do you have that patch somewhere?
Comment 4 Laurentiu Nicola 2016-01-26 06:08:34 UTC
Created attachment 121294 [details] [review]
Fix NULL dereference
Comment 5 Laurentiu Nicola 2016-01-26 06:15:27 UTC
(In reply to Jonas Ådahl from comment #3)
> (In reply to Laurentiu Nicola from comment #2)
> > Connecting with freerdp seems to work, at least with my patch applied.
> 
> Do you have that patch somewhere?

Sorry, what I meant is that even after fixing the crash here (see attached patch), weston still crashes with mstsc, while it works with wfreerdp. When I made that comment I didn't know whether connecting with wfreerdp worked without my initial patch.

I tested it in the meanwhile and:

1. the original code crashes with both wfreerdp and mstsc
2. with my fix, wfreerdp can connect, while mstsc can't:

[08:12:30.778] kbd_layout:0x409 kbd_type:0x7 kbd_subType:0x0 kbd_functionKeys:0xc
[08:12:30.778] xf_peer_activate: matching layout=us variant=(null)
[08:12:30.886] unable to checkDescriptor for 0x731010
[Thread 0x7fffe860a700 (LWP 18653) exited]
[Thread 0x7fffe7e09700 (LWP 18652) exited]
[Thread 0x7fffe7608700 (LWP 18651) exited]
[Thread 0x7fffe6e07700 (LWP 18650) exited]
[08:12:30.889] input_method disconnected, respawning...
[08:12:30.889] launching '/usr/lib/weston/weston-keyboard'
wl_registry@2: error 0: invalid global wl_seat (14)
[08:12:30.893] Error: /usr/lib/weston/weston-desktop-shell apparently cannot run at all.
Quitting...wl_registry@2: error 0: invalid global wl_seat (14)
[Inferior 1 (process 18640) exited normally]

3. there are a few other places in the same file that lack the same NULL check

I'm not familiar with the code, so I don't know whether focus_client should be NULL.

PS: while toying with it, I also got another crash:

Program received signal SIGSEGV, Segmentation fault.
weston_compositor_wake (compositor=compositor@entry=0x0) at ../src/compositor.c:3894
3894            uint32_t old_state = compositor->state;
(gdb) bt
#0  weston_compositor_wake (compositor=compositor@entry=0x0) at ../src/compositor.c:3894
#1  0x00000000004131c3 in notify_motion_absolute (seat=seat@entry=0x71c608, time=2089892085, x=x@entry=143872, y=y@entry=84992) at ../src/input.c:1281
#2  0x00007ffff61c4e3b in xf_mouseEvent (input=<optimized out>, flags=<optimized out>, x=<optimized out>, y=<optimized out>) at ../src/compositor-rdp.c:956
#3  0x00007ffff5f0f61c in fastpath_recv_inputs () from /usr/lib/libfreerdp.so.2.0
#4  0x00007ffff5f1f1b7 in ?? () from /usr/lib/libfreerdp.so.2.0
#5  0x00007ffff5f1f7a8 in ?? () from /usr/lib/libfreerdp.so.2.0
#6  0x00007ffff5f124f0 in transport_check_fds () from /usr/lib/libfreerdp.so.2.0
#7  0x00007ffff5f0b338 in rdp_check_fds () from /usr/lib/libfreerdp.so.2.0
#8  0x00007ffff5f1ef23 in ?? () from /usr/lib/libfreerdp.so.2.0
#9  0x00007ffff61c4c88 in rdp_client_activity (fd=<optimized out>, mask=<optimized out>, data=0x730ec0) at ../src/compositor-rdp.c:658
#10 0x00007ffff7bd2462 in wl_event_loop_dispatch () from /usr/lib/libwayland-server.so.0
#11 0x00007ffff7bd0cc5 in wl_display_run () from /usr/lib/libwayland-server.so.0
#12 0x000000000041c22b in main (argc=1, argv=0x7fffffffe4d8) at ../src/main.c:859
(gdb) l
3889     * Restarts the idle timer.
3890     */
3891    WL_EXPORT void
3892    weston_compositor_wake(struct weston_compositor *compositor)
3893    {
3894            uint32_t old_state = compositor->state;
3895
3896            /* The state needs to be changed before emitting the wake
3897             * signal because that may try to schedule a repaint which
3898             * will not work if the compositor is still sleeping */
(gdb) p compositor
$1 = (struct weston_compositor *) 0x0
Comment 6 Bryce Harrington 2016-05-23 22:26:19 UTC
Bumping to high since this is a reproducible crash bug.
Comment 7 Jonas Ådahl 2016-05-24 01:43:05 UTC
The original crash this bug reported was fixed back in January. Are the other issues still reproducible?
Comment 8 Laurentiu Nicola 2016-05-24 17:44:49 UTC
I have no idea, it kinda' stopped building. My FreeRDP version is 1:1.2.0_20160107-6.

../src/compositor-rdp.c: In function ‘xf_peer_activate’:
../src/compositor-rdp.c:58:29: error: too many arguments to function ‘rfx_context_reset’
 # define RFX_RESET(C, W, H) rfx_context_reset(C, W, H)
                             ^
../src/compositor-rdp.c:874:2: note: in expansion of macro ‘RFX_RESET’
  RFX_RESET(peerCtx->rfx_context, weston_output->width, weston_output->height);
  ^~~~~~~~~
In file included from /usr/include/freerdp/codecs.h:27:0,
                 from /usr/include/freerdp/freerdp.h:46,
                 from ../src/compositor-rdp.c:67:
/usr/include/freerdp/codec/rfx.h:186:18: note: declared here
 FREERDP_API void rfx_context_reset(RFX_CONTEXT* context);
                  ^~~~~~~~~~~~~~~~~
../src/compositor-rdp.c:57:29: error: too many arguments to function ‘nsc_context_reset’
 # define NSC_RESET(C, W, H) nsc_context_reset(C, W, H)
                             ^
../src/compositor-rdp.c:875:2: note: in expansion of macro ‘NSC_RESET’
  NSC_RESET(peerCtx->nsc_context, weston_output->width, weston_output->height);
  ^~~~~~~~~
In file included from /usr/include/freerdp/codecs.h:28:0,
                 from /usr/include/freerdp/freerdp.h:46,
                 from ../src/compositor-rdp.c:67:
/usr/include/freerdp/codec/nsc.h:96:17: note: declared here
 FREERDP_API int nsc_context_reset(NSC_CONTEXT* context);
                 ^~~~~~~~~~~~~~~~~
Makefile:4723: recipe for target 'src/rdp_backend_la-compositor-rdp.lo' failed
make[1]: *** [src/rdp_backend_la-compositor-rdp.lo] Error 1
Comment 9 Laurentiu Nicola 2016-07-05 13:45:56 UTC
Seems to work now.


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.