Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client/X11: Trying to paste copied files from a server crashes xfreerdp #10072

Closed
pnowack opened this issue Apr 14, 2024 · 9 comments
Closed
Assignees

Comments

@pnowack
Copy link
Member

pnowack commented Apr 14, 2024

Affected version master, also affects current 3.4.0 release.

Reproduction steps:

  1. Connect with xfreerdp3 to a server, that supports server-to-client file copy paste, e.g. g-r-d
  2. Copy the file on the server side
  3. Try to paste the file on the client side (e.g. in nautilus)

Upon switching to nautilus, xfreerdp fetches the file list and there immediately crashes with the following backtrace:

Program terminated with signal SIGABRT, Aborted.
#0  0x000076d82174432c in ?? () from /usr/lib/libc.so.6
[Current thread is 1 (Thread 0x76d7ebe006c0 (LWP 35690))]
(gdb) bt full
#0  0x000076d82174432c in ??? () at /usr/lib/libc.so.6
#1  0x000076d8216f36c8 in raise () at /usr/lib/libc.so.6
#2  0x000076d821c42856 in fatal_handler (signum=6) at /usr/src/debug/freerdp/FreeRDP/libfreerdp/utils/signal.c:138
        default_sigaction = {__sigaction_handler = {sa_handler = 0x0, sa_sigaction = 0x0}, sa_mask = {__val = {18446744067267100671, 0 <repeats 15 times>}}, sa_flags = 0, sa_restorer = 0x0}
        this_mask = {__val = {32, 0 <repeats 15 times>}}
        recursive = 1
        _log_cached_ptr = 0x60e9bdb97f70
        __func__ = "fatal_handler"
#3  0x000076d8216f3770 in <signal handler called> () at /usr/lib/libc.so.6
#4  0x000076d82174432c in ??? () at /usr/lib/libc.so.6
#5  0x000076d8216f36c8 in raise () at /usr/lib/libc.so.6
#6  0x000076d8216db4b8 in abort () at /usr/lib/libc.so.6
#7  0x000076d821b7764f in winpr_int_assert.constprop.25
    (condstr=condstr@entry=0x76d821b80691 "arrayList", fkt=fkt@entry=0x76d821bb4f00 <__func__.9.lto_priv.12> "ArrayList_Append", line=line@entry=316, file=0x76d821ba2d98 "/usr/src/debug/freerdp/FreeRDP/winpr/libwinpr/utils/collections/ArrayList.c") at /usr/src/debug/freerdp/FreeRDP/winpr/include/winpr/assert.h:48
        _log_cached_ptr = 0x76d7b40014e0
        __func__ = {<optimized out> <repeats 17 times>}
#8  0x000076d821b344d1 in ArrayList_Append (arrayList=<optimized out>, obj=<optimized out>) at /usr/src/debug/freerdp/FreeRDP/winpr/libwinpr/utils/collections/ArrayList.c:316
        index = <optimized out>
        rc = <optimized out>
        out = <optimized out>
        __func__ = {<optimized out> <repeats 17 times>}
#9  ArrayList_Append (arrayList=<optimized out>, obj=obj@entry=0x76d7b40012c0) at /usr/src/debug/freerdp/FreeRDP/winpr/libwinpr/utils/collections/ArrayList.c:311
        index = 0
        rc = 0
        __func__ = "ArrayList_Append"
#10 0x000076d821f44c1a in clip_data_dir_new (clip_data_id=<optimized out>, has_clip_data_id=1, file_context=0x76d8080bff50)
    at /usr/src/debug/freerdp/FreeRDP/client/common/client_cliprdr_file.c:1724
        root_dir = 0x76d8080fb3e0
        clip_data_dir = 0x76d7b40012c0
        path_length = 12
        root_dir = <optimized out>
        clip_data_dir = <optimized out>
        path_length = <optimized out>
        __func__ = {<optimized out> <repeats 18 times>}
#11 cliprdr_file_context_update_server_data (file_context=0x76d8080bff50, clip=0x76d808107510, data=data@entry=0x76d8080f9568, size=size@entry=596)
    at /usr/src/debug/freerdp/FreeRDP/client/common/client_cliprdr_file.c:1996
        clip_data_entry = 0x76d7b4000d00
        files = 0x76d7b4001060
        n_files = 1
--Type <RET> for more, q to quit, c to continue without paging--c
        __func__ = "cliprdr_file_context_update_server_data"
#12 0x000060e9bc7ef947 in xf_cliprdr_server_format_data_response (context=<optimized out>, formatDataResponse=<optimized out>)
    at /usr/src/debug/freerdp/FreeRDP/client/X11/xf_cliprdr.c:2063
        dstTargetFormat = <optimized out>
        bSuccess = 0
        pDstData = 0x0
        DstSize = 0
        SrcSize = 0
        srcFormatId = 0
        dstFormatId = 0
        nullTerminated = 0
        size = 596
        data = 0x76d8080f9568 "\001"
        xfc = 0x60e9bdb983e0
        clipboard = 0x76d8080e8ad0
        cached_data = 0x0
        format = <optimized out>
        willQuit = <optimized out>
        __func__ = "xf_cliprdr_server_format_data_response"
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
#13 0x000076d821f6c906 in cliprdr_process_format_data_response (msgFlags=<optimized out>, dataLen=<optimized out>, s=0x76d80854c100, cliprdr=<optimized out>)
    at /usr/src/debug/freerdp/FreeRDP/channels/cliprdr/client/cliprdr_format.c:238
        formatDataResponse = {common = {msgType = 5, msgFlags = 1, dataLen = 596}, requestedFormatData = 0x76d8080f9568 "\001"}
        context = 0x76d808132dc0
        error = <optimized out>
        mask = <optimized out>
        formatDataResponse = {common = {msgType = <optimized out>, msgFlags = <optimized out>, dataLen = <optimized out>}, requestedFormatData = <optimized out>}
        context = <optimized out>
        error = <optimized out>
        __func__ = {<optimized out> <repeats 37 times>}
        mask = <optimized out>
        _log_cached_ptr = <optimized out>
        _log_cached_ptr = <optimized out>
        _log_cached_ptr = <optimized out>
#14 cliprdr_order_recv (userdata=<optimized out>, s=0x76d80854c100) at /usr/src/debug/freerdp/FreeRDP/channels/cliprdr/client/cliprdr_main.c:517
        cliprdr = <optimized out>
        msgType = <optimized out>
        msgFlags = <optimized out>
        dataLen = <optimized out>
        error = 0
        __func__ = "cliprdr_order_recv"
        _log_cached_ptr = 0x76d7d8000b70
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
#15 0x000076d821f47efe in channel_client_thread_proc (userdata=0x76d808060200) at /usr/src/debug/freerdp/FreeRDP/channels/client/addin.c:560
        error = 0
        data = <optimized out>
        message = {id = 0, context = 0x0, wParam = 0x76d80854c100, lParam = 0x0, time = 1800624, Free = 0x0}
        internals = 0x76d808060200
        __func__ = "channel_client_thread_proc"
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
#16 0x000076d821b47e40 in thread_launcher (arg=0x76d808060d40) at /usr/src/debug/freerdp/FreeRDP/winpr/libwinpr/thread/thread.c:528
        rc = 0
        thread = 0x76d808060d40
        fkt = 0x76d821f47d60 <channel_client_thread_proc>
        exit = <optimized out>
        _log_cached_ptr = 0x0
        __func__ = "thread_launcher"
        _log_cached_ptr = 0x0
        _log_cached_ptr = 0x0
#17 0x000076d82174255a in ??? () at /usr/lib/libc.so.6
#18 0x000076d8217bfa3c in ??? () at /usr/lib/libc.so.6
(gdb) frame 10
#10 0x000076d821f44c1a in clip_data_dir_new (clip_data_id=<optimized out>, has_clip_data_id=1, file_context=0x76d8080bff50)
    at /usr/src/debug/freerdp/FreeRDP/client/common/client_cliprdr_file.c:1724
1724		if (!ArrayList_Append(root_dir->children, clip_data_dir))
(gdb) p root_dir
$1 = (CliprdrFuseFile *) 0x76d8080fb3e0
(gdb) p *root_dir
$2 = {parent = 0x0, children = 0x0, filename = 0x76d808012f30 "", filename_with_root = 0x75 <error: Cannot access memory at address 0x75>, list_idx = 0, 
  ino = 9223372036854775808, is_directory = 0, is_readonly = -2147483648, has_size = 0, size = 0, has_last_write_time = 0, last_write_time_unix = 0, has_clip_data_id = 0, 
  clip_data_id = 0}
(gdb)

It seems that something set the children list of the root_dir to NULL, which should not happen. I don't know what triggers that part, but this leads to xfreerdp crashing when trying to create a new FUSE directory (for the clip data id) under the FUSE root dir, since there the children list of root_dir is somehow NULL.

I've looked through the commit history of xf_cliprdr.c and client_cliprdr_file.c and couldn't find a fishy change since when I reworked the clipboard part. It did work, when I reworked that part, so not sure what introduced this regression here.
I could though reproduce this issue on multiple different machines without a single case, where the crash with the reproduction steps above, did not happen. Maybe we have some memory corruption here, though one, that always triggers.

@akallabeth
Copy link
Member

@pnowack ok, this is really a puzzle...
I can´t reproduce your issue here (connecting to a windows 10, copy some files/folders in explorer, paste locally in nautilus, debian 12)

@akallabeth akallabeth self-assigned this Apr 15, 2024
@pnowack
Copy link
Member Author

pnowack commented Apr 15, 2024

I did my test here with g-r-d-46 on the server side and with xfreerdp(3) on the client side in a GNOME 46 environment (both systems are Archlinux). I guess though a Ubuntu 24.04 LTS Beta or Fedora 40 Beta (with disabled SELinux) should work too as reproduction systems.

@akallabeth
Copy link
Member

@pnowack I´m currently preparing another batch of coverity issue fixes, might be part of these. (thread sync issue)

@pnowack
Copy link
Member Author

pnowack commented Apr 15, 2024

Not sure whether this can be tracked, but normally the only way, the children of the root_dir can be destroyed, is by removing the root_dir from the file_context->inode_table, which should only happen in cliprdr_file_context_free().
Currently, not at a place, where I can test this, but maybe we should check, whether the entry in the inode table with FUSE_ROOT_ID as key still exists after each operation.

Another idea could be an issue when we cast things, I think I saw something in the commit history that there were some changes regarding that.

@akallabeth
Copy link
Member

@pnowack can you do a bisect?
As I can´t reproduce currently it is hard to guess.
(or do you already have a last working version you can point to?)

@pnowack
Copy link
Member Author

pnowack commented Apr 15, 2024

@pnowack can you do a bisect?

Will try this week. It did work before christmas, when we had the 3.0 release there.

@akallabeth
Copy link
Member

hmm, did not really see any relevant changes since then :/
only thing #10076 the table should clean out the value if removed.

@pnowack
Copy link
Member Author

pnowack commented Apr 15, 2024

Ok, it looks like the source for this crash is external. I built now FreeRDP-3.0.0 (at that 3.0.0 tag) and I can reproduce the crash there as well.
However, when I built FreeRDP at christmas 2023, server-to-client file copy-paste did work just fine. It might be either gdb (using here version 13.2.1), or some (FreeRDP-)dependency seems to cause this issue here...externally. I don't know how

@akallabeth
Copy link
Member

closing this.
reopen if reproducible (and caused by FreeRDP)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants