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

v3: QPP interface + stringsearch, syscalls, and VMI #7

Draft
wants to merge 16 commits into
base: plugins_next
Choose a base branch
from

Conversation

AndrewFasano
Copy link

@AndrewFasano AndrewFasano commented Sep 10, 2024

Adds:

  1. our QPP interface to allow inter-plugin interactions
  2. Stringsearch plugin to search for strings being read/written
  3. Syscalls plugin to track system calls + syscalls logger plugin to log in various formats
  4. New callback on loadvm event
  5. New plugin method to translate guest virtual address to guest physical address
  6. Port PANDA's virtual machine introspection and integrate with syscalls logger

Rebased atop https://patchew.org/QEMU/[email protected]/ to use the qemu_plugin_read_memory_vaddr method. After these changes are merged, we can start sending these to the mailing list!

Rebased atop qemu main (v9.1.50) on Oct 24th.

Usage for VMI with the panda bionic qcow:

mkdir build
cd build
../configure --enable-system --disable-user --disable-docs --enable-plugins --enable-capstone
cd ..
./build/qemu-system-x86_64 \
        ~/.panda/bionic-server-cloudimg-amd64-noaslr-nokaslr.qcow2 \
        -nographic \
        -d plugin \
        -plugin build/contrib/plugins/libvmi_core.so \
        -plugin build/contrib/plugins/libvmi_linux.so,conf=contrib/plugins/vmi/linux/kernelinfo.conf,group=ubuntu:4.15.0-72-generic-noaslr-nokaslr:64 \
        -plugin build/contrib/plugins/libsyscalls.so \
        -plugin build/contrib/plugins/libsyscalls_logger.so \
        -m 1g

@AndrewFasano
Copy link
Author

Stringsearch seems to trigger some sort of deadlock which I can reproduce without our changes. Opened a bug in upstream https://gitlab.com/qemu-project/qemu/-/issues/2566

@AndrewFasano AndrewFasano force-pushed the qpp_pr3 branch 5 times, most recently from e2592a0 to 193c182 Compare September 12, 2024 21:32
Elysia Witham and others added 11 commits September 12, 2024 17:36
Signed-off-by: Elysia Witham <[email protected]>
In order for the QPP API to resolve interactions between plugins,
plugins must export their own names which cannot match any other
loaded plugins.

Signed-off-by: Elysia Witham <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
Plugin callbacks and their registered functions are stored in a
separate struct which is accessible from the plugin's ctx.
In order for plugins to use another plugin's callbacks, we have
internal functions that resolve a plugin's name to its ctx and
find a target plugin.

Signed-off-by: Elysia Witham <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
Plugins are able to use API functions which are explained in
<qemu-plugin.h> to create and run their own callbacks and register
functions on another plugin's callbacks.

Signed-off-by: Elysia Witham <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
Plugins can export functions or import functions from other
plugins using their name and the function name. This is also
described in <qemu-plugin.h>.

Signed-off-by: Elysia Witham <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
Plugins can use this macro in a header file which can be included
by both the exporting and importing plugins. The macro will
either use qemu_plugin_import_function to import the function
or just define it if the plugin is the same one that exports it.
If importing a function, "_qpp" will be appended to the end of the
function name.

Signed-off-by: Elysia Witham <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
These test plugins demonstrate the QPP API changes by exporting
and importing functions and creating and registering callbacks.
These tests are integrated into the `make check-tcg` tests.

This changes the check-tcg target to no longer have a one-to-one
correspondence to plugins in the tests/tcg/plugins directory as the
single qpp test involves both qpp_srv and qpp_client.

Signed-off-by: Elysia Witham <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
Plugin detects reads and writes of user-provided strings in memory. On
each detected string a QPP-style callback is triggered.

Co-authored-by: Jordan McLeod <[email protected]>
Signed-off-by: Andrew Fasano <[email protected]>
Detect syscalls and collect arguments for a few supported architectures.
Trigger QPP callbacks on each syscall.

Signed-off-by: Andrew Fasano <[email protected]>
Syscalls_logger consumes QPP events from the syscalls plugin and logs
this information in a variety of formats.

Signed-off-by: Andrew Fasano <[email protected]>
@AndrewFasano AndrewFasano force-pushed the qpp_pr3 branch 7 times, most recently from 39ab3fb to c80257c Compare September 13, 2024 18:42
Andrew Fasano added 2 commits September 13, 2024 14:43
@AndrewFasano AndrewFasano force-pushed the qpp_pr3 branch 3 times, most recently from f596515 to bbeb334 Compare September 13, 2024 18:57
@AndrewFasano AndrewFasano changed the title QPP PR v3 v3: QPP interface + stringsearch, syscalls, and VMI Sep 13, 2024
Andrew Fasano added 2 commits October 24, 2024 11:51
VMI provides a generic interface for virtual machine introspection
while vmi linux implements the backend for Linux guests

This code is a port of PANDA.re's operating system introspection code
named "VMI_linux"

Signed-off-by: Andrew Fasano <[email protected]>
lacraig2 pushed a commit that referenced this pull request Dec 3, 2024
Allow overlapping request by removing the assert that made it
impossible. There are only two callers:

1. block_copy_task_create()

It already asserts the very same condition before calling
reqlist_init_req().

2. cbw_snapshot_read_lock()

There is no need to have read requests be non-overlapping in
copy-before-write when used for snapshot-access. In fact, there was no
protection against two callers of cbw_snapshot_read_lock() calling
reqlist_init_req() with overlapping ranges and this could lead to an
assertion failure [1].

In particular, with the reproducer script below [0], two
cbw_co_snapshot_block_status() callers could race, with the second
calling reqlist_init_req() before the first one finishes and removes
its conflicting request.

[0]:

> #!/bin/bash -e
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=1024
> ./qemu-img create /tmp/fleecing.raw -f raw 1G
> (
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.raw \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
> {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
> {"execute": "nbd-server-start", "arguments": {"addr": { "type": "unix", "data": { "path": "/tmp/nbd.socket" } } } }
> {"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "snap0", "type": "nbd", "name": "exp0"}}
> EOF
> ) &
> sleep 5
> while true; do
> ./qemu-nbd -d /dev/nbd0
> ./qemu-nbd -c /dev/nbd0 nbd:unix:/tmp/nbd.socket:exportname=exp0 -f raw -r
> nbdinfo --map 'nbd+unix:///exp0?socket=/tmp/nbd.socket'
> done

[1]:

> #5  0x000071e5f0088eb2 in __GI___assert_fail (...) at ./assert/assert.c:101
> #6  0x0000615285438017 in reqlist_init_req (...) at ../block/reqlist.c:23
> #7  0x00006152853e2d98 in cbw_snapshot_read_lock (...) at ../block/copy-before-write.c:237
> #8  0x00006152853e3068 in cbw_co_snapshot_block_status (...) at ../block/copy-before-write.c:304
> #9  0x00006152853f4d22 in bdrv_co_snapshot_block_status (...) at ../block/io.c:3726
> qemu#10 0x000061528543a63e in snapshot_access_co_block_status (...) at ../block/snapshot-access.c:48
> qemu#11 0x00006152853f1a0a in bdrv_co_do_block_status (...) at ../block/io.c:2474
> qemu#12 0x00006152853f2016 in bdrv_co_common_block_status_above (...) at ../block/io.c:2652
> qemu#13 0x00006152853f22cf in bdrv_co_block_status_above (...) at ../block/io.c:2732
> qemu#14 0x00006152853d9a86 in blk_co_block_status_above (...) at ../block/block-backend.c:1473
> qemu#15 0x000061528538da6c in blockstatus_to_extents (...) at ../nbd/server.c:2374
> qemu#16 0x000061528538deb1 in nbd_co_send_block_status (...) at ../nbd/server.c:2481
> qemu#17 0x000061528538f424 in nbd_handle_request (...) at ../nbd/server.c:2978
> qemu#18 0x000061528538f906 in nbd_trip (...) at ../nbd/server.c:3121
> qemu#19 0x00006152855a7caf in coroutine_trampoline (...) at ../util/coroutine-ucontext.c:175

Cc: [email protected]
Suggested-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Fiona Ebner <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
lacraig2 pushed a commit that referenced this pull request Dec 3, 2024
In regime_is_user() we assert if we're passed an ARMMMUIdx_E10_*
mmuidx value. This used to make sense because we only used this
function in ptw.c and would never use it on this kind of stage 1+2
mmuidx, only for an individual stage 1 or stage 2 mmuidx.

However, when we implemented FEAT_E0PD we added a callsite in
aa64_va_parameters(), which means this can now be called for
stage 1+2 mmuidx values if the guest sets the TCG_ELX.{E0PD0,E0PD1}
bits to enable use of the feature. This will then result in
an assertion failure later, for instance on a TLBI operation:

#6  0x00007ffff6d0e70f in g_assertion_message_expr
    (domain=0x0, file=0x55555676eeba "../../target/arm/internals.h", line=978, func=0x555556771d48 <__func__.5> "regime_is_user", expr=<optimised out>)
    at ../../../glib/gtestutils.c:3279
#7  0x0000555555f286d2 in regime_is_user (env=0x555557f2fe00, mmu_idx=ARMMMUIdx_E10_0) at ../../target/arm/internals.h:978
#8  0x0000555555f3e31c in aa64_va_parameters (env=0x555557f2fe00, va=18446744073709551615, mmu_idx=ARMMMUIdx_E10_0, data=true, el1_is_aa32=false)
    at ../../target/arm/helper.c:12048
#9  0x0000555555f3163b in tlbi_aa64_get_range (env=0x555557f2fe00, mmuidx=ARMMMUIdx_E10_0, value=106721347371041) at ../../target/arm/helper.c:5214
qemu#10 0x0000555555f317e8 in do_rvae_write (env=0x555557f2fe00, value=106721347371041, idxmap=21, synced=true) at ../../target/arm/helper.c:5260
qemu#11 0x0000555555f31925 in tlbi_aa64_rvae1is_write (env=0x555557f2fe00, ri=0x555557fbeae0, value=106721347371041) at ../../target/arm/helper.c:5302
qemu#12 0x0000555556036f8f in helper_set_cp_reg64 (env=0x555557f2fe00, rip=0x555557fbeae0, value=106721347371041) at ../../target/arm/tcg/op_helper.c:965

Since we do know whether these mmuidx values are for usermode
or not, we can easily make regime_is_user() handle them:
ARMMMUIdx_E10_0 is user, and the other two are not.

Cc: [email protected]
Fixes: e4c93e4 ("target/arm: Implement FEAT_E0PD")
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
Reviewed-by: Alex Bennée <[email protected]>
Tested-by: Alex Bennée <[email protected]>
Message-id: [email protected]
lacraig2 pushed a commit that referenced this pull request Dec 3, 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
#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
#5  0x000000000cd36524 in process_event (opaque=0xf3c711a79000, obj=0xf3c711a4b000, err=0x0) at ../qga/main.c:677
#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
#7  0x000000000cd93860 in json_lexer_feed_char (lexer=lexer@entry=0xf3c711a79018, ch=125 '}', flush=flush@entry=false)
    at ../qobject/json-lexer.c:313
#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
#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 <[email protected]>
Reviewed-by: Konstantin Kostiuk <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Konstantin Kostiuk <[email protected]>
lacraig2 pushed a commit that referenced this pull request Dec 12, 2024
The released fix for this CVE:

  f6b0de5 ("9pfs: prevent opening special files (CVE-2023-2861)")

caused a regression with security_model=passthrough. When handling a
'Tmknod' request there was a side effect that 'Tmknod' request could fail
as 9p server was trying to adjust permissions:

  #6  close_if_special_file (fd=30) at ../hw/9pfs/9p-util.h:140
  #7  openat_file (mode=<optimized out>, flags=2228224,
      name=<optimized out>, dirfd=<optimized out>) at
      ../hw/9pfs/9p-util.h:181
  #8  fchmodat_nofollow (dirfd=dirfd@entry=31,
      name=name@entry=0x5555577ea6e0 "mysocket", mode=493) at
      ../hw/9pfs/9p-local.c:360
  #9  local_set_cred_passthrough (credp=0x7ffbbc4ace10, name=0x5555577ea6e0
      "mysocket", dirfd=31, fs_ctx=0x55555811f528) at
      ../hw/9pfs/9p-local.c:457
  qemu#10 local_mknod (fs_ctx=0x55555811f528, dir_path=<optimized out>,
      name=0x5555577ea6e0 "mysocket", credp=0x7ffbbc4ace10) at
      ../hw/9pfs/9p-local.c:702
  qemu#11 v9fs_co_mknod (pdu=pdu@entry=0x555558121140,
      fidp=fidp@entry=0x5555574c46c0, name=name@entry=0x7ffbbc4aced0,
      uid=1000, gid=1000, dev=<optimized out>, mode=49645,
      stbuf=0x7ffbbc4acef0) at ../hw/9pfs/cofs.c:205
  qemu#12 v9fs_mknod (opaque=0x555558121140) at ../hw/9pfs/9p.c:3711

That's because server was opening the special file to adjust permissions,
however it was using O_PATH and it would have not returned the file
descriptor to guest. So the call to close_if_special_file() on that branch
was incorrect.

Let's lift the restriction introduced by f6b0de5 such that it would
allow to open special files on host if O_PATH flag is supplied, not only
for 9p server's own operations as described above, but also for any client
'Topen' request.

It is safe to allow opening special files with O_PATH on host, because
O_PATH only allows path based operations on the resulting file descriptor
and prevents I/O such as read() and write() on that file descriptor.

Fixes: f6b0de5 ("9pfs: prevent opening special files (CVE-2023-2861)")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2337
Reported-by: Dirk Herrendorfer <[email protected]>
Signed-off-by: Christian Schoenebeck <[email protected]>
Reviewed-by: Greg Kurz <[email protected]>
Tested-by: Dirk Herrendorfer <[email protected]>
Message-Id: <[email protected]>
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.

1 participant