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

Ps whole regs #2

Open
wants to merge 7 commits into
base: development
Choose a base branch
from
Open

Conversation

PaoloS02
Copy link

No description provided.

@PaoloS02 PaoloS02 changed the base branch from master to development November 26, 2024 14:02
craigblackmore pushed a commit to craigblackmore/rise-rvv-tcg-qemu that referenced this pull request Dec 3, 2024
In extioi_setirq() we try to operate on a bit array stored as an
array of uint32_t using the set_bit() and clear_bit() functions
by casting the pointer to 'unsigned long *'.
This has two problems:
 * the alignment of 'uint32_t' is less than that of 'unsigned long'
   so we pass an insufficiently aligned pointer, which is
   undefined behaviour
 * on big-endian hosts the 64-bit 'unsigned long' will have
   its two halves the wrong way around, and we will produce
   incorrect results

The undefined behaviour is shown by the clang undefined-behaviour
sanitizer when running the loongarch64-virt functional test:

/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/bitops.h:41:5: runtime error: store to misaligned address 0x555559745d9c for type 'unsigned long', which requires 8 byte alignment
0x555559745d9c: note: pointer points here
  ff ff ff ff 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00
              ^
    #0 0x555556fb81c4 in set_bit /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/include/qemu/bitops.h:41:9
    embecosm#1 0x555556fb81c4 in extioi_setirq /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../hw/intc/loongarch_extioi.c:65:9
    embecosm#2 0x555556fb6e90 in pch_pic_irq_handler /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../hw/intc/loongarch_pch_pic.c:75:5
    qemu#3 0x555556710265 in serial_ioport_write /mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/clang/../../hw/char/serial.c

Fix these problems by using set_bit32() and clear_bit32(),
which work with bit arrays stored as an array of uint32_t.

Cc: [email protected]
Fixes: cbff2db ("hw/intc: Add LoongArch extioi interrupt controller(EIOINTC)")
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Reviewed-by: Bibo Mao <[email protected]>
Message-id: [email protected]
PaoloS02 pushed a commit to PaoloS02/rise-rvv-tcg-qemu that referenced this pull request Dec 18, 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
embecosm#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
embecosm#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
qemu#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 <[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]>
PaoloS02 pushed a commit to PaoloS02/rise-rvv-tcg-qemu that referenced this pull request Dec 18, 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
  embecosm#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
  embecosm#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
  qemu#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: [email protected]
Fixes: d62dbb5 ("virtio-9p: Add fidtype so that we can do type ...")
Reported-by: Akihiro Suda <[email protected]>
Tested-by: Akihiro Suda <[email protected]>
Signed-off-by: Christian Schoenebeck <[email protected]>
Reviewed-by: Greg Kurz <[email protected]>
Message-Id: <[email protected]>
This commit adds changes to the RISCV vector translation to call for
a specific node to perform a register offset based vector load
and by passing the relevant SEW information to the tcg function so
to perform correctly the load.
The aim for now is to reach correctness.
This should be applied next to stores.
Optimization will come later.
…reg rvv loads/stores.

This commit removes the custom tcg gvec nodes used to attempt to emulate
the whole register loads and stores and uses instead the tcg_gen_qemu_[ld,st]_i128
and tcg_gen_[st,ld]_i128 functions to load from memory to vector register
and viceversa when emulating a whole register vector load or store.

Whole register loads and store will always load and store at least
16 bytes but we need to add checks on atomicity for the host and
possibly endianness when calculating the memory addresses.

If necessary these loads and store can be broken into i64, i32, i16, i8.
PaoloS02 pushed a commit that referenced this pull request Jan 6, 2025
Found with test sbsaref introduced in [1].

[1] https://patchew.org/QEMU/[email protected]/

../block/vvfat.c:433:24: runtime error: index 14 out of bounds for type 'uint8_t [11]'
    #0 0x56151a66b93a in create_long_filename ../block/vvfat.c:433
    #1 0x56151a66f3d7 in create_short_and_long_name ../block/vvfat.c:725
    #2 0x56151a670403 in read_directory ../block/vvfat.c:804
    qemu#3 0x56151a674432 in init_directories ../block/vvfat.c:964
    qemu#4 0x56151a67867b in vvfat_open ../block/vvfat.c:1258
    qemu#5 0x56151a3b8e19 in bdrv_open_driver ../block.c:1660
    qemu#6 0x56151a3bb666 in bdrv_open_common ../block.c:1985
    qemu#7 0x56151a3cadb9 in bdrv_open_inherit ../block.c:4153
    qemu#8 0x56151a3c8850 in bdrv_open_child_bs ../block.c:3731
    qemu#9 0x56151a3ca832 in bdrv_open_inherit ../block.c:4098
    qemu#10 0x56151a3cbe40 in bdrv_open ../block.c:4248
    qemu#11 0x56151a46344f in blk_new_open ../block/block-backend.c:457
    qemu#12 0x56151a388bd9 in blockdev_init ../blockdev.c:612
    qemu#13 0x56151a38ab2d in drive_new ../blockdev.c:1006
    qemu#14 0x5615190fca41 in drive_init_func ../system/vl.c:649
    qemu#15 0x56151aa796dd in qemu_opts_foreach ../util/qemu-option.c:1135
    qemu#16 0x5615190fd2b6 in configure_blockdev ../system/vl.c:708
    qemu#17 0x56151910a307 in qemu_create_early_backends ../system/vl.c:2004
    qemu#18 0x561519113fcf in qemu_init ../system/vl.c:3685
    qemu#19 0x56151a7e438e in main ../system/main.c:47
    qemu#20 0x7f72d1a46249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    qemu#21 0x7f72d1a46304 in __libc_start_main_impl ../csu/libc-start.c:360
    qemu#22 0x561517e98510 in _start (/home/user/.work/qemu/build/qemu-system-aarch64+0x3b9b510)

The offset used can easily go beyond entry->name size. It's probably a
bug, but I don't have the time to dive into vfat specifics for now.

This change solves the ubsan issue, and is functionally equivalent, as
anything written past the entry->name array would not be read anyway.

Signed-off-by: Pierrick Bouvier <[email protected]>
Reviewed-by: Michael Tokarev <[email protected]>
Signed-off-by: Michael Tokarev <[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