On Donnerstag, 3. Februar 2022 07:20:05 CET Vitaly Chikunov wrote: > On Thu, Feb 03, 2022 at 07:55:41AM +0300, Vitaly Chikunov wrote: > > Christian, > > > > On Wed, Feb 02, 2022 at 05:55:45PM +0100, Christian Schoenebeck wrote: > > > On Freitag, 28. Januar 2022 23:33:26 CET Vitaly Chikunov wrote: > > > > `struct dirent' returned from readdir(3) could be shorter than > > > > `sizeof(struct dirent)', thus memcpy of sizeof length will overread > > > > > > > > into unallocated page causing SIGSEGV. Example stack trace: > > > > #0 0x00005555559ebeed v9fs_co_readdir_many > > > > (/usr/bin/qemu-system-x86_64 + > > > > > > > > 0x497eed) #1 0x00005555559ec2e9 v9fs_readdir > > > > (/usr/bin/qemu-system-x86_64 > > > > + 0x4982e9) #2 0x0000555555eb7983 coroutine_trampoline > > > > (/usr/bin/qemu-system-x86_64 + 0x963983) #3 0x00007ffff73e0be0 n/a > > > > (n/a + > > > > 0x0) > > > > > > > > While fixing, provide a helper for any future `struct dirent' cloning. > > > > > > > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/841 > > > > Cc: qemu-sta...@nongnu.org > > > > Co-authored-by: Christian Schoenebeck <qemu_...@crudebyte.com> > > > > Signed-off-by: Vitaly Chikunov <v...@altlinux.org> > > > > --- > > > > Tested on x86-64 Linux. > > > > > > I was too optimistic. Looks like this needs more work. With this patch > > > applied the 9p test cases [1] are crashing now: > > > > > > $ gdb --args tests/qtest/qos-test -m slow > > > ... > > > # Start of flush tests > > > ok 50 > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > > rtio-9p-tests/synth/flush/success ok 51 > > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > > rtio-9p-tests/synth/flush/ignored # End of flush tests > > > # Start of readdir tests > > > > I changed implementation from the one using dent->d_reclen to the one > > using > > > > strlen(dent->d_name) and it's passed readdir tests, but failed later: > > # Start of readdir tests > > ok 53 > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > rtio-9p-tests/synth/readdir/basic ok 54 > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > rtio-9p-tests/synth/readdir/split_512 ok 55 > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > rtio-9p-tests/synth/readdir/split_256 ok 56 > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > rtio-9p-tests/synth/readdir/split_128 # End of readdir tests > > # End of synth tests > > # Start of local tests > > # starting QEMU: exec x86_64-softmmu/qemu-system-x86_64 -qtest > > unix:/tmp/qtest-2822967.sock -qtest-log /dev/null -chardev > > socket,path=/tmp/qtest-2822967.qmp,id=char0 -mon > > chardev=char0,mode=control -display none -M pc -fsdev > > local,id=fsdev0,path='/usr/src/RPM/BUILD/qemu-6.2.0/build-dynamic/qtest > > -9p-local-NpcZCR',security_model=mapped-xattr -device > > virtio-9p-pci,fsdev=fsdev0,addr=04.0,mount_tag=qtest -accel qtest # > > GLib-DEBUG: setenv()/putenv() are not thread-safe and should not be > > used after threads are created ok 57 > > /x86_64/pc/i440FX-pcihost/pci-bus-pc/pci-bus/virtio-9p-pci/virtio-9p/vi > > rtio-9p-tests/local/config Received response 7 (RLERROR) instead of 73 > > (RMKDIR) > > Rlerror has errno 95 (Operation not supported) > > ** > > ERROR:../tests/qtest/virtio-9p-test.c:305:v9fs_req_recv: assertion > > failed (hdr.id == id): (7 == 73) Bail out! > > ERROR:../tests/qtest/virtio-9p-test.c:305:v9fs_req_recv: assertion > > failed (hdr.id == id): (7 == 73) Aborted > > I added some debugging output and in the bug case `d_reclen` is 0. Thus > this is not readdir's struct dirent, but something else (test-only > simulated dirent without accounting that we now have d_reclen logic). > > This maybe related to the other bug in > > static void synth_direntry(V9fsSynthNode *node, > struct dirent *entry, off_t off) > { > strcpy(entry->d_name, node->name); > entry->d_ino = node->attr->inode; > entry->d_off = off + 1; > } > > Where `d_reclen` is not updated.
The synth driver (used by the 'synth' 9p tests) intentionally just simulates a filesystem. The synth driver does not call any real fs syscalls, it just has its own very simple in-RAM-only structures that are used to simulate a fs and therefore the synth driver populates the dirent structure by itself. Could you try to resolve this issue in the synth driver and send a v3 of this patch? I am currently busy with other tasks right now. Best regards, Christian Schoenebeck
