Guys, are we playing with our sand-box toys or what? Can we apply this maybe to 1.5?? It's just insane that such a simple bugfixes, with lots of preceeding work to identify it, and with users suffering, are being simply ignored for months...
Thanks, /mjt 28.02.2013 13:12, Aneesh Kumar K.V wrote: > Michael Tokarev <m...@tls.msk.ru> writes: > >> When guest tries to chmod a block or char device file over 9pfs, >> the qemu process segfaults. >> >> On host: >> qemu-system-x86_64 -virtfs >> local,path=/dev,security_model=mapped-file,mount_tag=tag >> >> On guest: >> mount -t 9p -o trans=virtio tag /mnt >> chmod 0777 /mnt/tty > > any specific reason why you are trying 9p .u ? > >> >> Result (for 1.4.0): >> >> Program received signal SIGSEGV, Segmentation fault. >> 0x566095af in v9mode_to_mode (mode=8389101, extension=0xc7584ef8) >> at /build/kvm/git/hw/9pfs/virtio-9p.c:662 >> 662 if (extension && extension->data[0] == 'c') { >> (gdb) p *extension >> $1 = {size = 0, data = 0x0} >> (gdb) bt >> #0 0x566095af in v9mode_to_mode (mode=8389101, extension=0xc7584ef8) >> at hw/9pfs/virtio-9p.c:662 >> #1 0x5660f38b in v9fs_wstat (opaque=0xd250945c) >> at hw/9pfs/virtio-9p.c:2635 >> (gdb) frame 1 >> #1 0x5660f38b in v9fs_wstat (opaque=0xd250945c) >> at hw/9pfs/virtio-9p.c:2635 >> 2635 err = v9fs_co_chmod(pdu, &fidp->path, >> v9mode_to_mode(v9stat.mode, >> &v9stat.extension)); >> (gdb) p v9stat >> $2 = {size = 61, type = -1, dev = -1, qid = {type = -1 '\377', version = -1, >> path = -1}, mode = 8389101, atime = -1, mtime = -1, length = -1, name = { >> size = 0, data = 0x0}, uid = {size = 0, data = 0x0}, gid = {size = 0, >> data = 0x0}, muid = {size = 0, data = 0x0}, extension = {size = 0, >> data = 0x0}, n_uid = -1, n_gid = -1, n_muid = -1} >> >> >> Corresponding code in v9mode_to_mode(): >> >> if (mode & P9_STAT_MODE_DEVICE) { >> if (extension && extension->data[0] == 'c') { >> ret |= S_IFCHR; >> } else { >> ret |= S_IFBLK; >> } >> >> This (static) function (v9mode_to_mode) is called from only one place, >> namely from v9fs_wstat(), and it always calls it with non-NULL >> `extension' argument: &v9stat.extension. >> >> Maybe the buffer (extension->data) should be passed to it instead of >> the whole structure (extension)? Or the check be extended (or, >> since this function isn't called from any other place, _replaced_) to >> test for non-NULL ->data too? >> > > Thanks for the detailed analysis. Something like below ? > > diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c > index f526467..073067f 100644 > --- a/hw/9pfs/virtio-9p.c > +++ b/hw/9pfs/virtio-9p.c > @@ -659,7 +659,7 @@ static mode_t v9mode_to_mode(uint32_t mode, V9fsString > *extension) > ret |= S_IFIFO; > } > if (mode & P9_STAT_MODE_DEVICE) { > - if (extension && extension->data[0] == 'c') { > + if (extension->size && extension->data[0] == 'c') { > ret |= S_IFCHR; > } else { > ret |= S_IFBLK; > >