[PATCH 0/1] MR8642: server: Fix use-after-free in screen_buffer_destroy.
When an async object enters the async queue, its fd is released (to avoid a reference cycle I assume?). In screen_buffer_destroy, the screen_buffer's fd is released first, this cause it to be freed even when it could still be referenced by an async object in the queue. This means free_async_queue could use freed memory. * * * Found in ASan CI, but here is a much better report: <details> <summary>report</summary> ``` ================================================================= ==3803229==ERROR: AddressSanitizer: heap-use-after-free on address 0x5120006d6650 at pc 0x000000448b29 bp 0x7fff2dbcc5b0 sp 0x7fff2dbcc5a8 READ of size 8 at 0x5120006d6650 thread T0 #0 0x448b28 in fd_get_completion /home/shui/programs/wine/server/fd.c:2793 #1 0x4084f3 in free_async_queue /home/shui/programs/wine/server/async.c:231 #2 0x4228e4 in screen_buffer_destroy /home/shui/programs/wine/server/console.c:866 #3 0x478907 in release_object /home/shui/programs/wine/server/object.c:556 #4 0x421f13 in console_destroy /home/shui/programs/wine/server/console.c:768 #5 0x478907 in release_object /home/shui/programs/wine/server/object.c:556 #6 0x424d3a in console_connection_close_handle /home/shui/programs/wine/server/console.c:1285 #7 0x451498 in handle_table_destroy /home/shui/programs/wine/server/handle.c:185 #8 0x478907 in release_object /home/shui/programs/wine/server/object.c:556 #9 0x4515a2 in close_process_handles /home/shui/programs/wine/server/handle.c:198 #10 0x4805e5 in process_killed /home/shui/programs/wine/server/process.c:990 #11 0x480ab5 in remove_process_thread /home/shui/programs/wine/server/process.c:1040 #12 0x4fe26e in kill_thread /home/shui/programs/wine/server/thread.c:1574 #13 0x480f56 in kill_process /home/shui/programs/wine/server/process.c:1097 #14 0x47fbba in process_poll_event /home/shui/programs/wine/server/process.c:882 #15 0x43c954 in fd_poll_event /home/shui/programs/wine/server/fd.c:510 #16 0x43d338 in main_loop_epoll /home/shui/programs/wine/server/fd.c:618 #17 0x43dec3 in main_loop /home/shui/programs/wine/server/fd.c:972 #18 0x45e6a4 in main /home/shui/programs/wine/server/main.c:239 #19 0x7f4cf7f1d4d7 in __libc_start_call_main (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a4d7) (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) #20 0x7f4cf7f1d59a in __libc_start_main_alias_1 (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a59a) (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) #21 0x406a84 in _start (/home/shui/.cache/wine-builds/wow+asan/server/wineserver+0x406a84) 0x5120006d6650 is located 272 bytes inside of 288-byte region [0x5120006d6540,0x5120006d6660) freed by thread T0 here: #0 0x7f4cf81f73f8 in free.part.0 (/nix/store/6vzcxjxa2wlh3p9f5nhbk62bl3q313ri-gcc-14.3.0-lib/lib/libasan.so.8+0xfb3f8) #1 0x4776f5 in free_object /home/shui/programs/wine/server/object.c:333 #2 0x478913 in release_object /home/shui/programs/wine/server/object.c:557 #3 0x4228d4 in screen_buffer_destroy /home/shui/programs/wine/server/console.c:865 #4 0x478907 in release_object /home/shui/programs/wine/server/object.c:556 #5 0x421f13 in console_destroy /home/shui/programs/wine/server/console.c:768 #6 0x478907 in release_object /home/shui/programs/wine/server/object.c:556 #7 0x424d3a in console_connection_close_handle /home/shui/programs/wine/server/console.c:1285 #8 0x451498 in handle_table_destroy /home/shui/programs/wine/server/handle.c:185 #9 0x478907 in release_object /home/shui/programs/wine/server/object.c:556 #10 0x4515a2 in close_process_handles /home/shui/programs/wine/server/handle.c:198 #11 0x4805e5 in process_killed /home/shui/programs/wine/server/process.c:990 #12 0x480ab5 in remove_process_thread /home/shui/programs/wine/server/process.c:1040 #13 0x4fe26e in kill_thread /home/shui/programs/wine/server/thread.c:1574 #14 0x480f56 in kill_process /home/shui/programs/wine/server/process.c:1097 #15 0x47fbba in process_poll_event /home/shui/programs/wine/server/process.c:882 #16 0x43c954 in fd_poll_event /home/shui/programs/wine/server/fd.c:510 #17 0x43d338 in main_loop_epoll /home/shui/programs/wine/server/fd.c:618 #18 0x43dec3 in main_loop /home/shui/programs/wine/server/fd.c:972 #19 0x45e6a4 in main /home/shui/programs/wine/server/main.c:239 #20 0x7f4cf7f1d4d7 in __libc_start_call_main (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a4d7) (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) #21 0x7f4cf7f1d59a in __libc_start_main_alias_1 (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a59a) (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) #22 0x406a84 in _start (/home/shui/.cache/wine-builds/wow+asan/server/wineserver+0x406a84) previously allocated by thread T0 here: #0 0x7f4cf81f8757 in malloc (/nix/store/6vzcxjxa2wlh3p9f5nhbk62bl3q313ri-gcc-14.3.0-lib/lib/libasan.so.8+0xfc757) #1 0x476b07 in mem_alloc /home/shui/programs/wine/server/object.c:223 #2 0x47724d in alloc_object /home/shui/programs/wine/server/object.c:304 #3 0x442b6f in alloc_pseudo_fd /home/shui/programs/wine/server/fd.c:1747 #4 0x4214b0 in create_screen_buffer /home/shui/programs/wine/server/console.c:655 #5 0x422e92 in console_server_lookup_name /home/shui/programs/wine/server/console.c:930 #6 0x477b0b in lookup_named_object /home/shui/programs/wine/server/object.c:379 #7 0x4781c8 in open_named_object /home/shui/programs/wine/server/object.c:489 #8 0x449525 in req_open_file_object /home/shui/programs/wine/server/fd.c:2858 #9 0x4cea17 in call_req_handler /home/shui/programs/wine/server/request.c:305 #10 0x4cf082 in read_request /home/shui/programs/wine/server/request.c:360 #11 0x4f793a in thread_poll_event /home/shui/programs/wine/server/thread.c:581 #12 0x43c954 in fd_poll_event /home/shui/programs/wine/server/fd.c:510 #13 0x43d338 in main_loop_epoll /home/shui/programs/wine/server/fd.c:618 #14 0x43dec3 in main_loop /home/shui/programs/wine/server/fd.c:972 #15 0x45e6a4 in main /home/shui/programs/wine/server/main.c:239 #16 0x7f4cf7f1d4d7 in __libc_start_call_main (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a4d7) (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) #17 0x7f4cf7f1d59a in __libc_start_main_alias_1 (/nix/store/g2jzxk3s7cnkhh8yq55l4fbvf639zy37-glibc-2.40-66/lib/libc.so.6+0x2a59a) (BuildId: f117ee0f586dfa828cbdd08e37393c8f04f6480a) #18 0x406a84 in _start (/home/shui/.cache/wine-builds/wow+asan/server/wineserver+0x406a84) SUMMARY: AddressSanitizer: heap-use-after-free /home/shui/programs/wine/server/fd.c:2793 in fd_get_completion Shadow bytes around the buggy address: 0x5120006d6380: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00 0x5120006d6400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x5120006d6480: 00 00 00 00 00 00 00 00 00 00 00 00 fa fa fa fa 0x5120006d6500: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x5120006d6580: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x5120006d6600: fd fd fd fd fd fd fd fd fd fd[fd]fd fa fa fa fa 0x5120006d6680: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x5120006d6700: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x5120006d6780: fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa fa 0x5120006d6800: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd 0x5120006d6880: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb ==3803229==ABORTING ``` </details> I think this is the right fix? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8642
From: Yuxuan Shui <yshui(a)codeweavers.com> When an async object enters the async queue, its fd is released (to avoid a reference cycle I assume?). In screen_buffer_destroy, the screen_buffer's fd is released first, this cause it to be freed even when it could still be referenced by an async object in the queue. This means free_async_queue could use freed memory. --- server/console.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/console.c b/server/console.c index 8f01311892f..f0cb6689d4b 100644 --- a/server/console.c +++ b/server/console.c @@ -861,9 +861,9 @@ static void screen_buffer_destroy( struct object *obj ) queue_host_ioctl( screen_buffer->input->server, IOCTL_CONDRV_CLOSE_OUTPUT, screen_buffer->id, NULL, NULL ); } + free_async_queue( &screen_buffer->ioctl_q ); if (screen_buffer->sync) release_object( screen_buffer->sync ); if (screen_buffer->fd) release_object( screen_buffer->fd ); - free_async_queue( &screen_buffer->ioctl_q ); } static struct object *screen_buffer_open_file( struct object *obj, unsigned int access, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8642
participants (2)
-
Yuxuan Shui -
Yuxuan Shui (＠yshui)