Skip to content

Segfault with bpp = 32, and fastpath disabled #281

Closed
johneke opened this Issue Dec 16, 2011 · 10 comments

3 participants

@johneke
johneke commented Dec 16, 2011

Steps to reproduce:

./xfreerdp -a 32 --no-fastpath

Desktop connects, displays for a brief second, then crashes

Found in latest git pull as of 15th Dec, 2011

GDB and Valgrind backtraces (provided by Alexis Moinet)

(gdb) run -a 32 --no-fastpath -u username servername
Starting program: ./tmp/freerdp/builds/linux-x64-debug/client/X11/xfreerdp -a 32 --no-fastpath -u username servername
[Thread debugging using libthread_db enabled]
[New Thread 0x7ffff45d2700 (LWP 28504)]
connected to SERVERIP1
Password:
pduType bad
connected to SERVERIP2

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7ffff45d2700 (LWP 28504)]
0x0000000000410731 in xf_Pointer_Set (context=0x6c9bb0, pointer=0x0) at ./tmp/freerdp/client/X11/xf_graphics.c:197
197 XDefineCursor(xfi->display, xfi->window->handle, ((xfPointer*) pointer)->cursor);
(gdb) bt
#0 0x0000000000410731 in xf_Pointer_Set (context=0x6c9bb0, pointer=0x0) at ./tmp/freerdp/client/X11/xf_graphics.c:197
#1 0x00007ffff739210d in Pointer_Set (context=0x6c9bb0, pointer=0x0) at ./tmp/freerdp/libfreerdp-core/graphics.c:117
#2 0x00007ffff61dc695 in update_pointer_cached (context=0x6c9bb0, pointer_cached=0x6b5ff0) at ./tmp/freerdp/libfreerdp-cache/pointer.c:71
#3 0x00007ffff73a2003 in update_recv_pointer (update=0x6b49c0, s=0x7854a0) at ./tmp/freerdp/libfreerdp-core/update.c:238
#4 0x00007ffff739bec5 in rdp_recv_data_pdu (rdp=0x6a65f0, s=0x7854a0) at ./tmp/freerdp/libfreerdp-core/rdp.c:462
#5 0x00007ffff739c2f8 in rdp_recv_tpkt_pdu (rdp=0x6a65f0, s=0x7854a0) at ./tmp/freerdp/libfreerdp-core/rdp.c:663
#6 0x00007ffff739c461 in rdp_recv_pdu (rdp=0x6a65f0, s=0x7854a0) at ./tmp/freerdp/libfreerdp-core/rdp.c:709
#7 0x00007ffff739c5e5 in rdp_recv_callback (transport=0x6a8390, s=0x7854a0, extra=0x6a65f0) at ./tmp/freerdp/libfreerdp-core/rdp.c:764
#8 0x00007ffff73a0f7e in transport_check_fds (transport=0x6a8390) at ./tmp/freerdp/libfreerdp-core/transport.c:350
#9 0x00007ffff739c6c8 in rdp_check_fds (rdp=0x6a65f0) at ./tmp/freerdp/libfreerdp-core/rdp.c:795
#10 0x00007ffff7391b8a in freerdp_check_fds (instance=0x6a6460) at ./tmp/freerdp/libfreerdp-core/freerdp.c:122
#11 0x000000000041578e in xfreerdp_run (instance=0x6a6460) at ./tmp/freerdp/client/X11/xfreerdp.c:972
#12 0x00000000004158af in thread_func (param=0x6ca880) at ./tmp/freerdp/client/X11/xfreerdp.c:1009
#13 0x00007ffff53929ca in start_thread (arg=) at pthread_create.c:300
#14 0x00007ffff50ef70d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112
#15 0x0000000000000000 in ?? ()

valgrind report doesn't say more :

==31474== Invalid read of size 8
==31474== at 0x410731: xf_Pointer_Set (xf_graphics.c:197)
==31474== by 0x568410C: Pointer_Set (graphics.c:117)
==31474== by 0x682B694: update_pointer_cached (pointer.c:71)
==31474== by 0x5694002: update_recv_pointer (update.c:238)
==31474== by 0x568DEC4: rdp_recv_data_pdu (rdp.c:462)
==31474== by 0x568E2F7: rdp_recv_tpkt_pdu (rdp.c:663)
==31474== by 0x568E460: rdp_recv_pdu (rdp.c:709)
==31474== by 0x568E5E4: rdp_recv_callback (rdp.c:764)
==31474== by 0x5692F7D: transport_check_fds (transport.c:350)
==31474== by 0x568E6C7: rdp_check_fds (rdp.c:795)
==31474== by 0x5683B89: freerdp_check_fds (freerdp.c:122)
==31474== by 0x41578D: xfreerdp_run (xfreerdp.c:972)
==31474== Address 0xa0 is not stack'd, malloc'd or (recently) free'd

@awakecoding
FreeRDP member

I'm looking into it right now. It looks like the server sends a PTR_MSG_TYPE_POINTER message to create a pointer at cache index zero, to then send a PTR_MSG_TYPE_CACHED message to switch to pointer at index 0, but this pointer does not exist in the cache. Something's weird.

@awakecoding
FreeRDP member

@johneke I just fixed it by adding a check to see if the cached pointer exists. If it doesn't, I assume that the server just wants to use the default pointer, since it only happens once at connection time for index 0. This solves the segfault issue.

@celsius
celsius commented Dec 16, 2011

Although your fix probably solves the problem anyway, just a note :

I don't know exactly how the code works, but I made a step-by-step analysis with gdb when passing in function pointer_cache_get() (i.e. one function call just before a crash), and what I saw was :

  • index is not always 0
  • some entries of pointer_cache->entries seem to be valid pointer (at least they're not NULL)
  • index (almost ?) always choose a NULL with index pointer_cache->entries[index]

For instance I had cases where index was "4" and there were non-NULL pointer at entries[0], [1], [3], but entries[2] and entries[4] up to [31] were NULL (and thus pointer_cache_get() returned NULL)

I'm going to test with the fix, thanks

Alexis

@awakecoding awakecoding reopened this Dec 16, 2011
@awakecoding
FreeRDP member

@celsius ok, from what you just said, there might be a bigger issue. Which server are you using?

@celsius
celsius commented Dec 16, 2011

Windows 2008R2 (with redirection, but I also tested with direct connection)

@awakecoding
FreeRDP member

@celsius can you check if update_reset_state is called? I think this might have something to do with proper reactivation.

@celsius
celsius commented Dec 16, 2011

I just tested the fix, what happens is:

  • as long as the mouse stay on the desktop : the pointer is normal
  • if the mouse changes to some other pointer (resize, hand, "eggtimer (hourglass?)" )
  • if I move the mouse back to some place where the pointer should be normal, it remains a "special pointer", it never goes back to "normal pointer" (although it switches normally between all the others pointers)

EDIT: actually, no, it doesn't switch normally between the other pointers, some work, some don't, very random)

@awakecoding
FreeRDP member

@celsius: if I were to check for a null pointer, and then send a call to switch to the default pointer, would it fix the problem?

@celsius
celsius commented Dec 16, 2011

@awakecoding : update_reset_state is called just before the session opens (or at the beginning). I'm not sure switching to default pointer would fix the problem because I re-ran xfreerdp several times in the past minutes and I had one case where it was more or less stuck with the normal pointer (in that case I could not get the "char" pointer when hovering a form or the Windows Start menu search box)

@awakecoding
FreeRDP member

It turns out the real reason for this bug was that appended PDUs after an update PDU were only processed if they were update PDUs. The server would append other types of PDUs, such as pointer PDUs, after an update PDU and those were discarded only in slow-path mode because of this bug. I just fixed the issue by parsing all potential PDU types that could be appended to an update PDU. This might fix other bugs as well.

@awakecoding awakecoding closed this Jan 8, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.