Skip to content

ui/cocoa: Update Cocoa UI from akihikodaki/qemu #2

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

Closed
wants to merge 4 commits into from

Conversation

brad-x
Copy link

@brad-x brad-x commented Jan 3, 2023

This merges most of the changes to QEMU Cocoa UI on the macos branch by @akihikodaki with three exceptions:

  • Excluded ba64a79 (main-loop: introduce WITH_QEMU_IOTHREAD_LOCK)
  • d0059be (input: Allow to choose console with qemu_input_is_absolute)
    • Including this would be great but the scope was beyond a simple update to the cocoa UI, open to suggestions here. Would mean updates to all other UI's
  • Added an ifdef around line 2540 to support builds without OpenGL enabled (otherwise you get an undeclared identifier)

@github-actions github-actions bot closed this Jan 3, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2023
@osy osy reopened this Jan 3, 2023
@utmapp utmapp unlocked this conversation Jan 3, 2023
@utmapp utmapp deleted a comment from github-actions bot Jan 3, 2023
@osy osy marked this pull request as draft January 3, 2023 16:57
@brad-x
Copy link
Author

brad-x commented Jan 3, 2023

High level notes, to be extended with specific references. Not necessarily in proper order yet

Some additional work will need to be done here to:

  • Merge relevant changes from more recent versions of the Cocoa UI from mainline QEMU
  • Ensure akihikodaki/qemu/ui/cocoa.m with the above changes properly receives infrastructure changes from mainline QEMU and the utmapp fork
  • Examine changes to utmapp/qemu/ui/cocoa/*.m where these diverged from the akihikodaki repo and merge these as well

@osy
Copy link

osy commented May 1, 2023

The latest merge included all the changes from akihikodaki/qemu

@osy osy closed this May 1, 2023
osy pushed a commit that referenced this pull request Nov 24, 2024
Since commit e99441a ("ui/curses: Do not use console_select()")
qemu_text_console_put_keysym() no longer checks for NULL console
argument, which leads to a later crash:

Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault.
0x00005555559ee186 in qemu_text_console_handle_keysym (s=0x0, keysym=31) at ../ui/console-vc.c:332
332	        } else if (s->echo && (keysym == '\r' || keysym == '\n')) {
(gdb) bt
 #0  0x00005555559ee186 in qemu_text_console_handle_keysym (s=0x0, keysym=31) at ../ui/console-vc.c:332
 #1  0x00005555559e18e5 in qemu_text_console_put_keysym (s=<optimized out>, keysym=<optimized out>) at ../ui/console.c:303
 #2  0x00005555559f2e88 in do_key_event (vs=vs@entry=0x5555579045c0, down=down@entry=1, keycode=keycode@entry=60, sym=sym@entry=65471) at ../ui/vnc.c:2034
 #3  0x00005555559f845c in ext_key_event (vs=0x5555579045c0, down=1, sym=65471, keycode=<optimized out>) at ../ui/vnc.c:2070
 qemu#4  protocol_client_msg (vs=0x5555579045c0, data=<optimized out>, len=<optimized out>) at ../ui/vnc.c:2514
 qemu#5  0x00005555559f515c in vnc_client_read (vs=0x5555579045c0) at ../ui/vnc.c:1607

Fixes: e99441a ("ui/curses: Do not use console_select()")
Fixes: https://issues.redhat.com/browse/RHEL-50529
Cc: qemu-stable@nongnu.org
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
(cherry picked from commit 0e60fc8)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
osy pushed a commit that referenced this pull request Nov 24, 2024
When SET_STREAM_FORMAT is called, we should clear the existing setup.

Factor out common function to close a stream.

Direct leak of 144 byte(s) in 3 object(s) allocated from:
    #0 0x7f91d38f7350 in calloc (/lib64/libasan.so.8+0xf7350) (BuildId: a4ad7eb954b390cf00f07fa10952988a41d9fc7a)
    #1 0x7f91d2ab7871 in g_malloc0 (/lib64/libglib-2.0.so.0+0x64871) (BuildId: 36b60dbd02e796145a982d0151ce37202ec05649)
    #2 0x562fa2f447ee in timer_new_full /home/elmarco/src/qemu/include/qemu/timer.h:538
    #3 0x562fa2f4486f in timer_new /home/elmarco/src/qemu/include/qemu/timer.h:559
    qemu#4 0x562fa2f448a9 in timer_new_ns /home/elmarco/src/qemu/include/qemu/timer.h:577
    qemu#5 0x562fa2f47955 in hda_audio_setup ../hw/audio/hda-codec.c:490
    qemu#6 0x562fa2f4897e in hda_audio_command ../hw/audio/hda-codec.c:605

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-ID: <20241008125028.1177932-3-marcandre.lureau@redhat.com>
(cherry picked from commit 6d6e233)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
osy pushed a commit that referenced this pull request Nov 24, 2024
qemu-ga on a NetBSD -current VM terminates with a SIGSEGV upon receiving
'guest-set-time' command...

Core was generated by `qemu-ga'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000cd37a40 in ga_pipe_read_str (fd=fd@entry=0xffffff922a20, str=str@entry=0xffffff922a18)
    at ../qga/commands-posix.c:88
88	        *str[len] = '\0';
[Current thread is 1 (process 1112)]
(gdb) bt
#0  0x000000000cd37a40 in ga_pipe_read_str (fd=fd@entry=0xffffff922a20, str=str@entry=0xffffff922a18)
    at ../qga/commands-posix.c:88
#1  0x000000000cd37b60 in ga_run_command (argv=argv@entry=0xffffff922a90,
    action=action@entry=0xcda34b8 "set hardware clock to system time", errp=errp@entry=0xffffff922a70, in_str=0x0)
    at ../qga/commands-posix.c:164
#2  0x000000000cd380c4 in qmp_guest_set_time (has_time=<optimized out>, time_ns=<optimized out>,
    errp=errp@entry=0xffffff922ad0) at ../qga/commands-posix.c:304
#3  0x000000000cd253d8 in qmp_marshal_guest_set_time (args=<optimized out>, ret=<optimized out>, errp=0xffffff922b48)
    at qga/qga-qapi-commands.c:193
qemu#4  0x000000000cd4e71c in qmp_dispatch (cmds=cmds@entry=0xcdf5b18 <ga_commands>, request=request@entry=0xf3c711a4b000,
    allow_oob=allow_oob@entry=false, cur_mon=cur_mon@entry=0x0) at ../qapi/qmp-dispatch.c:220
qemu#5  0x000000000cd36524 in process_event (opaque=0xf3c711a79000, obj=0xf3c711a4b000, err=0x0) at ../qga/main.c:677
qemu#6  0x000000000cd526f0 in json_message_process_token (lexer=lexer@entry=0xf3c711a79018, input=0xf3c712072480,
    type=type@entry=JSON_RCURLY, x=28, y=1) at ../qobject/json-streamer.c:99
qemu#7  0x000000000cd93860 in json_lexer_feed_char (lexer=lexer@entry=0xf3c711a79018, ch=125 '}', flush=flush@entry=false)
    at ../qobject/json-lexer.c:313
qemu#8  0x000000000cd93a00 in json_lexer_feed (lexer=lexer@entry=0xf3c711a79018,
    buffer=buffer@entry=0xffffff922d10 "{\"execute\":\"guest-set-time\"}\n", size=<optimized out>)
    at ../qobject/json-lexer.c:350
qemu#9  0x000000000cd5290c in json_message_parser_feed (parser=parser@entry=0xf3c711a79000,
    buffer=buffer@entry=0xffffff922d10 "{\"execute\":\"guest-set-time\"}\n", size=<optimized out>)
    at ../qobject/json-streamer.c:121
qemu#10 0x000000000cd361fc in channel_event_cb (condition=<optimized out>, data=0xf3c711a79000) at ../qga/main.c:703
qemu#11 0x000000000cd3710c in ga_channel_client_event (channel=<optimized out>, condition=<optimized out>, data=0xf3c711b2d300)
    at ../qga/channel-posix.c:94
qemu#12 0x0000f3c7120d9bec in g_main_dispatch () from /usr/pkg/lib/libglib-2.0.so.0
qemu#13 0x0000f3c7120dd25c in g_main_context_iterate_unlocked.constprop () from /usr/pkg/lib/libglib-2.0.so.0
qemu#14 0x0000f3c7120ddbf0 in g_main_loop_run () from /usr/pkg/lib/libglib-2.0.so.0
qemu#15 0x000000000cda00d8 in run_agent_once (s=0xf3c711a79000) at ../qga/main.c:1522
qemu#16 run_agent (s=0xf3c711a79000) at ../qga/main.c:1559
qemu#17 main (argc=<optimized out>, argv=<optimized out>) at ../qga/main.c:1671
(gdb)

The commandline options used on the host machine...
qemu-system-aarch64 \
   -machine type=virt,pflash0=rom \
   -m 8G \
   -cpu host \
   -smp 8 \
   -accel hvf \
   -device virtio-net-pci,netdev=unet \
   -device virtio-blk-pci,drive=hd \
   -drive file=netbsd.qcow2,if=none,id=hd \
   -netdev user,id=unet,hostfwd=tcp::2223-:22 \
   -object rng-random,filename=/dev/urandom,id=viornd0 \
   -device virtio-rng-pci,rng=viornd0 \
   -serial mon:stdio \
   -display none \
   -blockdev node-name=rom,driver=file,filename=/opt/homebrew/Cellar/qemu/9.0.2/share/qemu/edk2-aarch64-code.fd,read-only=true \
   -chardev socket,path=/tmp/qga_netbsd.sock,server=on,wait=off,id=qga0 \
   -device virtio-serial \
   -device virtconsole,chardev=qga0,name=org.qemu.guest_agent.0

This patch rectifies the operator precedence while assigning the NUL
terminator.

Fixes: c3f32c1

Signed-off-by: Sunil Nimmagadda <sunil@nimmagadda.net>
Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Link: https://lore.kernel.org/r/m15xppk9qg.fsf@nimmagadda.net
Signed-off-by: Konstantin Kostiuk <kkostiuk@redhat.com>
(cherry picked from commit 9cfe110)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
osy pushed a commit that referenced this pull request Nov 24, 2024
A bad (broken or malicious) 9p client (guest) could cause QEMU host to
crash by sending a 9p 'Treaddir' request with a numeric file ID (FID) that
was previously opened for a file instead of an expected directory:

  #0  0x0000762aff8f4919 in __GI___rewinddir (dirp=0xf) at
    ../sysdeps/unix/sysv/linux/rewinddir.c:29
  #1  0x0000557b7625fb40 in do_readdir_many (pdu=0x557bb67d2eb0,
    fidp=0x557bb67955b0, entries=0x762afe9fff58, offset=0, maxsize=131072,
    dostat=<optimized out>) at ../hw/9pfs/codir.c:101
  #2  v9fs_co_readdir_many (pdu=pdu@entry=0x557bb67d2eb0,
    fidp=fidp@entry=0x557bb67955b0, entries=entries@entry=0x762afe9fff58,
    offset=0, maxsize=131072, dostat=false) at ../hw/9pfs/codir.c:226
  #3  0x0000557b7625c1f9 in v9fs_do_readdir (pdu=0x557bb67d2eb0,
    fidp=0x557bb67955b0, offset=<optimized out>,
    max_count=<optimized out>) at ../hw/9pfs/9p.c:2488
  qemu#4  v9fs_readdir (opaque=0x557bb67d2eb0) at ../hw/9pfs/9p.c:2602

That's because V9fsFidOpenState was declared as union type. So the
same memory region is used for either an open POSIX file handle (int),
or a POSIX DIR* pointer, etc., so 9p server incorrectly used the
previously opened (valid) POSIX file handle (0xf) as DIR* pointer,
eventually causing a crash in glibc's rewinddir() function.

Root cause was therefore a missing check in 9p server's 'Treaddir'
request handler, which must ensure that the client supplied FID was
really opened as directory stream before trying to access the
aforementioned union and its DIR* member.

Cc: qemu-stable@nongnu.org
Fixes: d62dbb5 ("virtio-9p: Add fidtype so that we can do type ...")
Reported-by: Akihiro Suda <suda.kyoto@gmail.com>
Tested-by: Akihiro Suda <suda.kyoto@gmail.com>
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <E1t8GnN-002RS8-E2@kylie.crudebyte.com>
(cherry picked from commit 042b4eb)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
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

Successfully merging this pull request may close these issues.

2 participants