[Qemu-devel] [PATCH] Fix bug with virtio-9p fsync

[Sassan Panahinejad]

v9fs_fsync and possibly others break when asked to operate on a directory.
It does not check fid_type to see if it is operating on a directory and 
therefore accesses the wrong element of the fs union.
This error can result in guest applications failing (in my case it was dpkg).
This patch fixes the issue, although there may be other, similar bugs in 
virtio-9p.

Signed-off-by: Sassan Panahinejad <address@hidden>
---

Here I've implemented it as a function. If you'd prefer a macro or inline 
function then let me know.

Thanks
Sassan

 hw/virtio-9p.c |   43 +++++++++++++++++++++++++++++++++++++------
 1 files changed, 37 insertions(+), 6 deletions(-)

diff --git a/hw/virtio-9p.c b/hw/virtio-9p.c
index 7e29535..26be0fc 100644
--- a/hw/virtio-9p.c
+++ b/hw/virtio-9p.c
@@ -1860,6 +1860,18 @@ static void v9fs_post_do_fsync(V9fsState *s, V9fsPDU 
*pdu, int err)
     complete_pdu(s, pdu, err);
 }
 
+static int v9fs_get_fd(V9fsFidState *fidp) 
+{
+    switch (fidp->fid_type) {
+        case P9_FID_FILE: 
+            return fidp->fs.fd;
+        case P9_FID_DIR: 
+            return dirfd(fidp->fs.dir);
+        default:
+            return -1;
+    }
+}
+
 static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
 {
     int32_t fid;
@@ -1867,6 +1879,7 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
     V9fsFidState *fidp;
     int datasync;
     int err;
+    int fd;
 
     pdu_unmarshal(pdu, offset, "dd", &fid, &datasync);
     fidp = lookup_fid(s, fid);
@@ -1875,7 +1888,11 @@ static void v9fs_fsync(V9fsState *s, V9fsPDU *pdu)
         v9fs_post_do_fsync(s, pdu, err);
         return;
     }
-    err = v9fs_do_fsync(s, fidp->fs.fd, datasync);
+    if ((fd = v9fs_get_fd(fidp)) >= 0) {
+        err = v9fs_do_fsync(s, fd, datasync);
+    } else {
+       err = -EINVAL;
+    }
     v9fs_post_do_fsync(s, pdu, err);
 }
 
@@ -2984,6 +3001,7 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
     int32_t fid;
     V9fsWstatState *vs;
     int err = 0;
+    int fd;
 
     vs = qemu_malloc(sizeof(*vs));
     vs->pdu = pdu;
@@ -2999,7 +3017,10 @@ static void v9fs_wstat(V9fsState *s, V9fsPDU *pdu)
 
     /* do we need to sync the file? */
     if (donttouch_stat(&vs->v9stat)) {
-        err = v9fs_do_fsync(s, vs->fidp->fs.fd, 0);
+        if ((fd = v9fs_get_fd(vs->fidp)) >= 0) {
+            err = v9fs_do_fsync(s, fd, 0);
+        } else
+            err = -EINVAL;
         v9fs_wstat_post_fsync(s, vs, err);
         return;
     }
@@ -3176,6 +3197,7 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
 {
     int32_t fid, err = 0;
     V9fsLockState *vs;
+    int fd;
 
     vs = qemu_mallocz(sizeof(*vs));
     vs->pdu = pdu;
@@ -3198,8 +3220,12 @@ static void v9fs_lock(V9fsState *s, V9fsPDU *pdu)
         err = -ENOENT;
         goto out;
     }
-
-    err = v9fs_do_fstat(s, vs->fidp->fs.fd, &vs->stbuf);
+    if ((fd = v9fs_get_fd(vs->fidp)) >= 0) {
+        err = v9fs_do_fstat(s, fd, &vs->stbuf);
+    } else {
+        err = -EINVAL;
+        goto out;
+    }
     if (err < 0) {
         err = -errno;
         goto out;
@@ -3221,6 +3247,7 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
 {
     int32_t fid, err = 0;
     V9fsGetlockState *vs;
+    int fd;
 
     vs = qemu_mallocz(sizeof(*vs));
     vs->pdu = pdu;
@@ -3236,8 +3263,12 @@ static void v9fs_getlock(V9fsState *s, V9fsPDU *pdu)
         err = -ENOENT;
         goto out;
     }
-
-    err = v9fs_do_fstat(s, vs->fidp->fs.fd, &vs->stbuf);
+    if ((fd = v9fs_get_fd(vs->fidp)) >= 0) {
+        err = v9fs_do_fstat(s, fd, &vs->stbuf);
+    } else {
+        err = -EINVAL;
+        goto out;
+    }
     if (err < 0) {
         err = -errno;
         goto out;
-- 
1.7.0.4