All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
@ 2019-07-03 10:55 ` Christian Schoenebeck via Qemu-devel
  2019-07-31 19:14   ` Greg Kurz
  2019-07-03 11:01 ` [Qemu-devel] [PATCH v5 2/5] 9p: Treat multiple devices on one export as an error Christian Schoenebeck via Qemu-devel
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-03 10:55 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Greg Kurz, Antonios Motakis

There is no need for signedness on these QID fields for 9p.

Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
[CS: - Also make QID type unsigned.
     - Adjust donttouch_stat() to new types.
     - Adjust trace-events to new types. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 fsdev/9p-marshal.h   |  6 +++---
 hw/9pfs/9p.c         |  6 +++---
 hw/9pfs/trace-events | 14 +++++++-------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
index c8823d878f..8f3babb60a 100644
--- a/fsdev/9p-marshal.h
+++ b/fsdev/9p-marshal.h
@@ -9,9 +9,9 @@ typedef struct V9fsString
 
 typedef struct V9fsQID
 {
-    int8_t type;
-    int32_t version;
-    int64_t path;
+    uint8_t type;
+    uint32_t version;
+    uint64_t path;
 } V9fsQID;
 
 typedef struct V9fsStat
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 55821343e5..586a6dccba 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -743,9 +743,9 @@ static int donttouch_stat(V9fsStat *stat)
 {
     if (stat->type == -1 &&
         stat->dev == -1 &&
-        stat->qid.type == -1 &&
-        stat->qid.version == -1 &&
-        stat->qid.path == -1 &&
+        stat->qid.type == 0xff &&
+        stat->qid.version == (uint32_t) -1 &&
+        stat->qid.path == (uint64_t) -1 &&
         stat->mode == -1 &&
         stat->atime == -1 &&
         stat->mtime == -1 &&
diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
index c0a0a4ab5d..10188daf7f 100644
--- a/hw/9pfs/trace-events
+++ b/hw/9pfs/trace-events
@@ -6,7 +6,7 @@ v9fs_rerror(uint16_t tag, uint8_t id, int err) "tag %d id %d err %d"
 v9fs_version(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
 v9fs_version_return(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
 v9fs_attach(uint16_t tag, uint8_t id, int32_t fid, int32_t afid, char* uname, char* aname) "tag %u id %u fid %d afid %d uname %s aname %s"
-v9fs_attach_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path) "tag %d id %d type %d version %d path %"PRId64
+v9fs_attach_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path) "tag %u id %u type %u version %u path %"PRIu64
 v9fs_stat(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
 v9fs_stat_return(uint16_t tag, uint8_t id, int32_t mode, int32_t atime, int32_t mtime, int64_t length) "tag %d id %d stat={mode %d atime %d mtime %d length %"PRId64"}"
 v9fs_getattr(uint16_t tag, uint8_t id, int32_t fid, uint64_t request_mask) "tag %d id %d fid %d request_mask %"PRIu64
@@ -14,9 +14,9 @@ v9fs_getattr_return(uint16_t tag, uint8_t id, uint64_t result_mask, uint32_t mod
 v9fs_walk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, uint16_t nwnames) "tag %d id %d fid %d newfid %d nwnames %d"
 v9fs_walk_return(uint16_t tag, uint8_t id, uint16_t nwnames, void* qids) "tag %d id %d nwnames %d qids %p"
 v9fs_open(uint16_t tag, uint8_t id, int32_t fid, int32_t mode) "tag %d id %d fid %d mode %d"
-v9fs_open_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} iounit %d"
+v9fs_open_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d"
 v9fs_lcreate(uint16_t tag, uint8_t id, int32_t dfid, int32_t flags, int32_t mode, uint32_t gid) "tag %d id %d dfid %d flags %d mode %d gid %u"
-v9fs_lcreate_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int32_t iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} iounit %d"
+v9fs_lcreate_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int32_t iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d"
 v9fs_fsync(uint16_t tag, uint8_t id, int32_t fid, int datasync) "tag %d id %d fid %d datasync %d"
 v9fs_clunk(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
 v9fs_read(uint16_t tag, uint8_t id, int32_t fid, uint64_t off, uint32_t max_count) "tag %d id %d fid %d off %"PRIu64" max_count %u"
@@ -26,21 +26,21 @@ v9fs_readdir_return(uint16_t tag, uint8_t id, uint32_t count, ssize_t retval) "t
 v9fs_write(uint16_t tag, uint8_t id, int32_t fid, uint64_t off, uint32_t count, int cnt) "tag %d id %d fid %d off %"PRIu64" count %u cnt %d"
 v9fs_write_return(uint16_t tag, uint8_t id, int32_t total, ssize_t err) "tag %d id %d total %d err %zd"
 v9fs_create(uint16_t tag, uint8_t id, int32_t fid, char* name, int32_t perm, int8_t mode) "tag %d id %d fid %d name %s perm %d mode %d"
-v9fs_create_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} iounit %d"
+v9fs_create_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d"
 v9fs_symlink(uint16_t tag, uint8_t id, int32_t fid,  char* name, char* symname, uint32_t gid) "tag %d id %d fid %d name %s symname %s gid %u"
-v9fs_symlink_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path) "tag %d id %d qid={type %d version %d path %"PRId64"}"
+v9fs_symlink_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path) "tag %u id %u qid={type %u version %u path %"PRIu64"}"
 v9fs_flush(uint16_t tag, uint8_t id, int16_t flush_tag) "tag %d id %d flush_tag %d"
 v9fs_link(uint16_t tag, uint8_t id, int32_t dfid, int32_t oldfid, char* name) "tag %d id %d dfid %d oldfid %d name %s"
 v9fs_remove(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
 v9fs_wstat(uint16_t tag, uint8_t id, int32_t fid, int32_t mode, int32_t atime, int32_t mtime) "tag %u id %u fid %d stat={mode %d atime %d mtime %d}"
 v9fs_mknod(uint16_t tag, uint8_t id, int32_t fid, int mode, int major, int minor) "tag %d id %d fid %d mode %d major %d minor %d"
-v9fs_mknod_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path) "tag %d id %d qid={type %d version %d path %"PRId64"}"
+v9fs_mknod_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path) "tag %u id %u qid={type %u version %u path %"PRIu64"}"
 v9fs_lock(uint16_t tag, uint8_t id, int32_t fid, uint8_t type, uint64_t start, uint64_t length) "tag %d id %d fid %d type %d start %"PRIu64" length %"PRIu64
 v9fs_lock_return(uint16_t tag, uint8_t id, int8_t status) "tag %d id %d status %d"
 v9fs_getlock(uint16_t tag, uint8_t id, int32_t fid, uint8_t type, uint64_t start, uint64_t length)"tag %d id %d fid %d type %d start %"PRIu64" length %"PRIu64
 v9fs_getlock_return(uint16_t tag, uint8_t id, uint8_t type, uint64_t start, uint64_t length, uint32_t proc_id) "tag %d id %d type %d start %"PRIu64" length %"PRIu64" proc_id %u"
 v9fs_mkdir(uint16_t tag, uint8_t id, int32_t fid, char* name, int mode, uint32_t gid) "tag %u id %u fid %d name %s mode %d gid %u"
-v9fs_mkdir_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int err) "tag %u id %u qid={type %d version %d path %"PRId64"} err %d"
+v9fs_mkdir_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int err) "tag %u id %u qid={type %u version %u path %"PRIu64"} err %d"
 v9fs_xattrwalk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, char* name) "tag %d id %d fid %d newfid %d name %s"
 v9fs_xattrwalk_return(uint16_t tag, uint8_t id, int64_t size) "tag %d id %d size %"PRId64
 v9fs_xattrcreate(uint16_t tag, uint8_t id, int32_t fid, char* name, uint64_t size, int flags) "tag %d id %d fid %d name %s size %"PRIu64" flags %d"
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v5 2/5] 9p: Treat multiple devices on one export as an error
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
  2019-07-03 10:55 ` [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path Christian Schoenebeck via Qemu-devel
@ 2019-07-03 11:01 ` Christian Schoenebeck via Qemu-devel
  2019-07-31 20:30   ` Greg Kurz
  2019-07-03 11:13 ` [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes' Christian Schoenebeck via Qemu-devel
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-03 11:01 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Greg Kurz, Antonios Motakis

The QID path should uniquely identify a file. However, the
inode of a file is currently used as the QID path, which
on its own only uniquely identifies files within a device.
Here we track the device hosting the 9pfs share, in order
to prevent security issues with QID path collisions from
other devices.

Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
[CS: - Assign dev_id to export root's device already in
       v9fs_device_realize_common(), not postponed in
       stat_to_qid().
     - error_report_once() if more than one device was
       shared by export.
     - Return -ENODEV instead of -ENOSYS in stat_to_qid().
     - Fixed typo in log comment. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++------------
 hw/9pfs/9p.h |  1 +
 2 files changed, 56 insertions(+), 14 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 586a6dccba..8cc65c2c67 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -572,10 +572,18 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
                                 P9_STAT_MODE_SOCKET)
 
 /* This is the algorithm from ufs in spfs */
-static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
+static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
 {
     size_t size;
 
+    if (pdu->s->dev_id != stbuf->st_dev) {
+        error_report_once(
+            "9p: Multiple devices detected in same VirtFS export. "
+            "You must use a separate export for each device."
+        );
+        return -ENODEV;
+    }
+
     memset(&qidp->path, 0, sizeof(qidp->path));
     size = MIN(sizeof(stbuf->st_ino), sizeof(qidp->path));
     memcpy(&qidp->path, &stbuf->st_ino, size);
@@ -587,6 +595,8 @@ static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
     if (S_ISLNK(stbuf->st_mode)) {
         qidp->type |= P9_QID_TYPE_SYMLINK;
     }
+
+    return 0;
 }
 
 static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
@@ -599,7 +609,10 @@ static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
     if (err < 0) {
         return err;
     }
-    stat_to_qid(&stbuf, qidp);
+    err = stat_to_qid(pdu, &stbuf, qidp);
+    if (err < 0) {
+        return err;
+    }
     return 0;
 }
 
@@ -830,7 +843,10 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,
 
     memset(v9stat, 0, sizeof(*v9stat));
 
-    stat_to_qid(stbuf, &v9stat->qid);
+    err = stat_to_qid(pdu, stbuf, &v9stat->qid);
+    if (err < 0) {
+        return err;
+    }
     v9stat->mode = stat_to_v9mode(stbuf);
     v9stat->atime = stbuf->st_atime;
     v9stat->mtime = stbuf->st_mtime;
@@ -891,7 +907,7 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,
 #define P9_STATS_ALL           0x00003fffULL /* Mask for All fields above */
 
 
-static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
+static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
                                 V9fsStatDotl *v9lstat)
 {
     memset(v9lstat, 0, sizeof(*v9lstat));
@@ -913,7 +929,7 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
     /* Currently we only support BASIC fields in stat */
     v9lstat->st_result_mask = P9_STATS_BASIC;
 
-    stat_to_qid(stbuf, &v9lstat->qid);
+    return stat_to_qid(pdu, stbuf, &v9lstat->qid);
 }
 
 static void print_sg(struct iovec *sg, int cnt)
@@ -1115,7 +1131,6 @@ static void coroutine_fn v9fs_getattr(void *opaque)
     uint64_t request_mask;
     V9fsStatDotl v9stat_dotl;
     V9fsPDU *pdu = opaque;
-    V9fsState *s = pdu->s;
 
     retval = pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
     if (retval < 0) {
@@ -1136,7 +1151,10 @@ static void coroutine_fn v9fs_getattr(void *opaque)
     if (retval < 0) {
         goto out;
     }
-    stat_to_v9stat_dotl(s, &stbuf, &v9stat_dotl);
+    retval = stat_to_v9stat_dotl(pdu, &stbuf, &v9stat_dotl);
+    if (retval < 0) {
+        goto out;
+    }
 
     /*  fill st_gen if requested and supported by underlying fs */
     if (request_mask & P9_STATS_GEN) {
@@ -1381,7 +1399,10 @@ static void coroutine_fn v9fs_walk(void *opaque)
             if (err < 0) {
                 goto out;
             }
-            stat_to_qid(&stbuf, &qid);
+            err = stat_to_qid(pdu, &stbuf, &qid);
+            if (err < 0) {
+                goto out;
+            }
             v9fs_path_copy(&dpath, &path);
         }
         memcpy(&qids[name_idx], &qid, sizeof(qid));
@@ -1483,7 +1504,10 @@ static void coroutine_fn v9fs_open(void *opaque)
     if (err < 0) {
         goto out;
     }
-    stat_to_qid(&stbuf, &qid);
+    err = stat_to_qid(pdu, &stbuf, &qid);
+    if (err < 0) {
+        goto out;
+    }
     if (S_ISDIR(stbuf.st_mode)) {
         err = v9fs_co_opendir(pdu, fidp);
         if (err < 0) {
@@ -1593,7 +1617,10 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
         fidp->flags |= FID_NON_RECLAIMABLE;
     }
     iounit =  get_iounit(pdu, &fidp->path);
-    stat_to_qid(&stbuf, &qid);
+    err = stat_to_qid(pdu, &stbuf, &qid);
+    if (err < 0) {
+        goto out;
+    }
     err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
     if (err < 0) {
         goto out;
@@ -2327,7 +2354,10 @@ static void coroutine_fn v9fs_create(void *opaque)
         }
     }
     iounit = get_iounit(pdu, &fidp->path);
-    stat_to_qid(&stbuf, &qid);
+    err = stat_to_qid(pdu, &stbuf, &qid);
+    if (err < 0) {
+        goto out;
+    }
     err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
     if (err < 0) {
         goto out;
@@ -2384,7 +2414,10 @@ static void coroutine_fn v9fs_symlink(void *opaque)
     if (err < 0) {
         goto out;
     }
-    stat_to_qid(&stbuf, &qid);
+    err = stat_to_qid(pdu, &stbuf, &qid);
+    if (err < 0) {
+        goto out;
+    }
     err =  pdu_marshal(pdu, offset, "Q", &qid);
     if (err < 0) {
         goto out;
@@ -3064,7 +3097,10 @@ static void coroutine_fn v9fs_mknod(void *opaque)
     if (err < 0) {
         goto out;
     }
-    stat_to_qid(&stbuf, &qid);
+    err = stat_to_qid(pdu, &stbuf, &qid);
+    if (err < 0) {
+        goto out;
+    }
     err = pdu_marshal(pdu, offset, "Q", &qid);
     if (err < 0) {
         goto out;
@@ -3222,7 +3258,10 @@ static void coroutine_fn v9fs_mkdir(void *opaque)
     if (err < 0) {
         goto out;
     }
-    stat_to_qid(&stbuf, &qid);
+    err = stat_to_qid(pdu, &stbuf, &qid);
+    if (err < 0) {
+        goto out;
+    }
     err = pdu_marshal(pdu, offset, "Q", &qid);
     if (err < 0) {
         goto out;
@@ -3633,6 +3672,8 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
         goto out;
     }
 
+    s->dev_id = stat.st_dev;
+
     s->ctx.fst = &fse->fst;
     fsdev_throttle_init(s->ctx.fst);
 
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 8883761b2c..5e316178d5 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -256,6 +256,7 @@ struct V9fsState
     Error *migration_blocker;
     V9fsConf fsconf;
     V9fsQID root_qid;
+    dev_t dev_id;
 };
 
 /* 9p2000.L open flags */
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes'
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
  2019-07-03 10:55 ` [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path Christian Schoenebeck via Qemu-devel
  2019-07-03 11:01 ` [Qemu-devel] [PATCH v5 2/5] 9p: Treat multiple devices on one export as an error Christian Schoenebeck via Qemu-devel
@ 2019-07-03 11:13 ` Christian Schoenebeck via Qemu-devel
  2019-07-06 10:22   ` Christian Schoenebeck via Qemu-devel
  2019-07-03 11:17 ` [Qemu-devel] [PATCH v5 4/5] 9p: stat_to_qid: implement slow path Christian Schoenebeck via Qemu-devel
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-03 11:13 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Greg Kurz, Antonios Motakis

To support multiple devices on the 9p share, and avoid
qid path collisions we take the device id as input
to generate a unique QID path. The lowest 48 bits of
the path will be set equal to the file inode, and the
top bits will be uniquely assigned based on the top
16 bits of the inode and the device id.

Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
[CS: - Rebased to master head.
     - Updated hash calls to new xxhash API.
     - Added virtfs option 'remap_inodes', original patch
       simply did the inode remapping without being asked.
     - Updated docs for new option 'remap_inodes'.
     - Capture root_ino in v9fs_device_realize_common() as
       well, not just the device id.
     - Fixed v9fs_do_readdir() having exposed info outside
       export root with '..' entry.
     - Fixed v9fs_do_readdir() not having remapped inodes.
     - Log error message when running out of prefixes in
       qid_path_prefixmap().
     - Fixed definition of QPATH_INO_MASK.
     - Dropped unnecessary parantheses in qpp_lookup_func().
     - Dropped unnecessary g_malloc0() result checks. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 fsdev/file-op-9p.h      |   1 +
 fsdev/qemu-fsdev-opts.c |   7 +-
 fsdev/qemu-fsdev.c      |   6 ++
 hw/9pfs/9p.c            | 196 +++++++++++++++++++++++++++++++++++++++++++-----
 hw/9pfs/9p.h            |  13 ++++
 qemu-options.hx         |  25 ++++--
 vl.c                    |   3 +
 7 files changed, 224 insertions(+), 27 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index c757c8099f..6c1663c252 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -59,6 +59,7 @@ typedef struct ExtendedOps {
 #define V9FS_RDONLY                 0x00000040
 #define V9FS_PROXY_SOCK_FD          0x00000080
 #define V9FS_PROXY_SOCK_NAME        0x00000100
+#define V9FS_REMAP_INODES           0x00000200
 
 #define V9FS_SEC_MASK               0x0000003C
 
diff --git a/fsdev/qemu-fsdev-opts.c b/fsdev/qemu-fsdev-opts.c
index 7c31ffffaf..64a8052266 100644
--- a/fsdev/qemu-fsdev-opts.c
+++ b/fsdev/qemu-fsdev-opts.c
@@ -31,7 +31,9 @@ static QemuOptsList qemu_fsdev_opts = {
         }, {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
-
+        }, {
+            .name = "remap_inodes",
+            .type = QEMU_OPT_BOOL,
         }, {
             .name = "socket",
             .type = QEMU_OPT_STRING,
@@ -76,6 +78,9 @@ static QemuOptsList qemu_virtfs_opts = {
             .name = "readonly",
             .type = QEMU_OPT_BOOL,
         }, {
+            .name = "remap_inodes",
+            .type = QEMU_OPT_BOOL,
+        }, {
             .name = "socket",
             .type = QEMU_OPT_STRING,
         }, {
diff --git a/fsdev/qemu-fsdev.c b/fsdev/qemu-fsdev.c
index 077a8c4e2b..b6fa9799be 100644
--- a/fsdev/qemu-fsdev.c
+++ b/fsdev/qemu-fsdev.c
@@ -121,6 +121,7 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp)
     const char *fsdev_id = qemu_opts_id(opts);
     const char *fsdriver = qemu_opt_get(opts, "fsdriver");
     const char *writeout = qemu_opt_get(opts, "writeout");
+    bool remap_inodes = qemu_opt_get_bool(opts, "remap_inodes", 0);
     bool ro = qemu_opt_get_bool(opts, "readonly", 0);
 
     if (!fsdev_id) {
@@ -161,6 +162,11 @@ int qemu_fsdev_add(QemuOpts *opts, Error **errp)
     } else {
         fsle->fse.export_flags &= ~V9FS_RDONLY;
     }
+    if (remap_inodes) {
+        fsle->fse.export_flags |= V9FS_REMAP_INODES;
+    } else {
+        fsle->fse.export_flags &= ~V9FS_REMAP_INODES;
+    }
 
     if (fsle->fse.ops->parse_opts) {
         if (fsle->fse.ops->parse_opts(opts, &fsle->fse, errp)) {
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 8cc65c2c67..39c6c2a894 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -25,6 +25,7 @@
 #include "trace.h"
 #include "migration/blocker.h"
 #include "sysemu/qtest.h"
+#include "qemu/xxhash.h"
 
 int open_fd_hw;
 int total_open_fd;
@@ -571,22 +572,98 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
                                 P9_STAT_MODE_NAMED_PIPE |   \
                                 P9_STAT_MODE_SOCKET)
 
-/* This is the algorithm from ufs in spfs */
+
+/* creative abuse of tb_hash_func7, which is based on xxhash */
+static uint32_t qpp_hash(QppEntry e)
+{
+    return qemu_xxhash7(e.ino_prefix, e.dev, 0, 0, 0);
+}
+
+static bool qpp_lookup_func(const void *obj, const void *userp)
+{
+    const QppEntry *e1 = obj, *e2 = userp;
+    return e1->dev == e2->dev && e1->ino_prefix == e2->ino_prefix;
+}
+
+static void qpp_table_remove(void *p, uint32_t h, void *up)
+{
+    g_free(p);
+}
+
+static void qpp_table_destroy(struct qht *ht)
+{
+    qht_iter(ht, qpp_table_remove, NULL);
+    qht_destroy(ht);
+}
+
+/* stat_to_qid needs to map inode number (64 bits) and device id (32 bits)
+ * to a unique QID path (64 bits). To avoid having to map and keep track
+ * of up to 2^64 objects, we map only the 16 highest bits of the inode plus
+ * the device id to the 16 highest bits of the QID path. The 48 lowest bits
+ * of the QID path equal to the lowest bits of the inode number.
+ *
+ * This takes advantage of the fact that inode number are usually not
+ * random but allocated sequentially, so we have fewer items to keep
+ * track of.
+ */
+static int qid_path_prefixmap(V9fsPDU *pdu, const struct stat *stbuf,
+                                uint64_t *path)
+{
+    QppEntry lookup = {
+        .dev = stbuf->st_dev,
+        .ino_prefix = (uint16_t) (stbuf->st_ino >> 48)
+    }, *val;
+    uint32_t hash = qpp_hash(lookup);
+
+    val = qht_lookup(&pdu->s->qpp_table, &lookup, hash);
+
+    if (!val) {
+        if (pdu->s->qp_prefix_next == 0) {
+            /* we ran out of prefixes */
+            error_report_once(
+                "9p: No more prefixes available for remapping inodes from "
+                "host to guest."
+            );
+            return -ENFILE;
+        }
+
+        val = g_malloc0(sizeof(QppEntry));
+        *val = lookup;
+
+        /* new unique inode prefix and device combo */
+        val->qp_prefix = pdu->s->qp_prefix_next++;
+        qht_insert(&pdu->s->qpp_table, val, hash, NULL);
+    }
+
+    *path = ((uint64_t)val->qp_prefix << 48) | (stbuf->st_ino & QPATH_INO_MASK);
+    return 0;
+}
+
 static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
 {
+    int err;
     size_t size;
 
-    if (pdu->s->dev_id != stbuf->st_dev) {
-        error_report_once(
-            "9p: Multiple devices detected in same VirtFS export. "
-            "You must use a separate export for each device."
-        );
-        return -ENODEV;
+    if (pdu->s->ctx.export_flags & V9FS_REMAP_INODES) {
+        /* map inode+device to qid path (fast path) */
+        err = qid_path_prefixmap(pdu, stbuf, &qidp->path);
+        if (err) {
+            return err;
+        }
+    } else {
+        if (pdu->s->dev_id != stbuf->st_dev) {
+            error_report_once(
+                "9p: Multiple devices detected in same VirtFS export. "
+                "You must either use a separate export for each device "
+                "shared from host or enable virtfs option 'remap_inodes'."
+            );
+            return -ENODEV;
+        }
+        memset(&qidp->path, 0, sizeof(qidp->path));
+        size = MIN(sizeof(stbuf->st_ino), sizeof(qidp->path));
+        memcpy(&qidp->path, &stbuf->st_ino, size);
     }
 
-    memset(&qidp->path, 0, sizeof(qidp->path));
-    size = MIN(sizeof(stbuf->st_ino), sizeof(qidp->path));
-    memcpy(&qidp->path, &stbuf->st_ino, size);
     qidp->version = stbuf->st_mtime ^ (stbuf->st_size << 8);
     qidp->type = 0;
     if (S_ISDIR(stbuf->st_mode)) {
@@ -616,6 +693,30 @@ static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
     return 0;
 }
 
+static int coroutine_fn dirent_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
+                                      struct dirent *dent, V9fsQID *qidp)
+{
+    struct stat stbuf;
+    V9fsPath path;
+    int err;
+
+    v9fs_path_init(&path);
+
+    err = v9fs_co_name_to_path(pdu, &fidp->path, dent->d_name, &path);
+    if (err < 0) {
+        goto out;
+    }
+    err = v9fs_co_lstat(pdu, &path, &stbuf);
+    if (err < 0) {
+        goto out;
+    }
+    err = stat_to_qid(pdu, &stbuf, qidp);
+
+out:
+    v9fs_path_free(&path);
+    return err;
+}
+
 V9fsPDU *pdu_alloc(V9fsState *s)
 {
     V9fsPDU *pdu = NULL;
@@ -1940,6 +2041,19 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
     int32_t count = 0;
     off_t saved_dir_pos;
     struct dirent *dent;
+    struct stat stbuf;
+    bool fidIsExportRoot;
+
+    /*
+     * determine if fidp is the export root, which is required for safe
+     * handling of ".." below
+     */
+    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
+    if (err < 0) {
+        return err;
+    }
+    fidIsExportRoot = pdu->s->dev_id == stbuf.st_dev &&
+                      pdu->s->root_ino == stbuf.st_ino;
 
     /* save the directory position */
     saved_dir_pos = v9fs_co_telldir(pdu, fidp);
@@ -1964,16 +2078,51 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
             v9fs_string_free(&name);
             return count;
         }
-        /*
-         * Fill up just the path field of qid because the client uses
-         * only that. To fill the entire qid structure we will have
-         * to stat each dirent found, which is expensive
-         */
-        size = MIN(sizeof(dent->d_ino), sizeof(qid.path));
-        memcpy(&qid.path, &dent->d_ino, size);
-        /* Fill the other fields with dummy values */
-        qid.type = 0;
-        qid.version = 0;
+
+        if (fidIsExportRoot && !strcmp("..", dent->d_name)) {
+            /*
+             * if "." is export root, then return qid of export root for
+             * ".." to avoid exposing anything outside the export
+             */
+            err = fid_to_qid(pdu, fidp, &qid);
+            if (err < 0) {
+                v9fs_readdir_unlock(&fidp->fs.dir);
+                v9fs_co_seekdir(pdu, fidp, saved_dir_pos);
+                v9fs_string_free(&name);
+                return err;
+            }
+        } else if (pdu->s->ctx.export_flags & V9FS_REMAP_INODES) {
+            /*
+             * dirent_to_qid() implies expensive stat call for each entry,
+             * we must do that here though since inode remapping requires
+             * the device id, which in turn might be different for
+             * different entries; we cannot make any assumption to avoid
+             * that here.
+             */
+            err = dirent_to_qid(pdu, fidp, dent, &qid);
+            if (err < 0) {
+                v9fs_readdir_unlock(&fidp->fs.dir);
+                v9fs_co_seekdir(pdu, fidp, saved_dir_pos);
+                v9fs_string_free(&name);
+                return err;
+            }
+        } else {
+            /*
+             * Fill up just the path field of qid because the client uses
+             * only that. To fill the entire qid structure we will have
+             * to stat each dirent found, which is expensive. For the
+             * latter reason we don't call dirent_to_qid() here. Only drawback
+             * is that no multi-device export detection of stat_to_qid()
+             * would be done and provided as error to the user here. But
+             * user would get that error anyway when accessing those
+             * files/dirs through other ways.
+             */
+            size = MIN(sizeof(dent->d_ino), sizeof(qid.path));
+            memcpy(&qid.path, &dent->d_ino, size);
+            /* Fill the other fields with dummy values */
+            qid.type = 0;
+            qid.version = 0;
+        }
 
         /* 11 = 7 + 4 (7 = start offset, 4 = space for storing count) */
         len = pdu_marshal(pdu, 11 + count, "Qqbs",
@@ -3672,8 +3821,13 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
         goto out;
     }
 
+    s->root_ino = stat.st_ino;
     s->dev_id = stat.st_dev;
 
+    /* QID path hash table. 1 entry ought to be enough for anybody ;) */
+    qht_init(&s->qpp_table, qpp_lookup_func, 1, QHT_MODE_AUTO_RESIZE);
+    s->qp_prefix_next = 1; /* reserve 0 to detect overflow */
+
     s->ctx.fst = &fse->fst;
     fsdev_throttle_init(s->ctx.fst);
 
@@ -3687,6 +3841,7 @@ out:
         }
         g_free(s->tag);
         g_free(s->ctx.fs_root);
+        qpp_table_destroy(&s->qpp_table);
         v9fs_path_free(&path);
     }
     return rc;
@@ -3699,6 +3854,7 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
     }
     fsdev_throttle_cleanup(s->ctx.fst);
     g_free(s->tag);
+    qpp_table_destroy(&s->qpp_table);
     g_free(s->ctx.fs_root);
 }
 
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 5e316178d5..a283b0193e 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -8,6 +8,7 @@
 #include "fsdev/9p-iov-marshal.h"
 #include "qemu/thread.h"
 #include "qemu/coroutine.h"
+#include "qemu/qht.h"
 
 enum {
     P9_TLERROR = 6,
@@ -235,6 +236,15 @@ struct V9fsFidState
     V9fsFidState *rclm_lst;
 };
 
+#define QPATH_INO_MASK        ((1ULL << 48) - 1)
+
+/* QID path prefix entry, see stat_to_qid */
+typedef struct {
+    dev_t dev;
+    uint16_t ino_prefix;
+    uint16_t qp_prefix;
+} QppEntry;
+
 struct V9fsState
 {
     QLIST_HEAD(, V9fsPDU) free_list;
@@ -256,7 +266,10 @@ struct V9fsState
     Error *migration_blocker;
     V9fsConf fsconf;
     V9fsQID root_qid;
+    ino_t root_ino;
     dev_t dev_id;
+    struct qht qpp_table;
+    uint16_t qp_prefix_next;
 };
 
 /* 9p2000.L open flags */
diff --git a/qemu-options.hx b/qemu-options.hx
index c18b79099a..08823e6e9a 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1334,17 +1334,17 @@ ETEXI
 
 DEF("virtfs", HAS_ARG, QEMU_OPTION_virtfs,
     "-virtfs local,path=path,mount_tag=tag,security_model=mapped-xattr|mapped-file|passthrough|none\n"
-    "        [,id=id][,writeout=immediate][,readonly][,fmode=fmode][,dmode=dmode]\n"
-    "-virtfs proxy,mount_tag=tag,socket=socket[,id=id][,writeout=immediate][,readonly]\n"
-    "-virtfs proxy,mount_tag=tag,sock_fd=sock_fd[,id=id][,writeout=immediate][,readonly]\n"
+    "        [,id=id][,writeout=immediate][,readonly][,fmode=fmode][,dmode=dmode][,remap_inodes]\n"
+    "-virtfs proxy,mount_tag=tag,socket=socket[,id=id][,writeout=immediate][,readonly][,remap_inodes]\n"
+    "-virtfs proxy,mount_tag=tag,sock_fd=sock_fd[,id=id][,writeout=immediate][,readonly][,remap_inodes]\n"
     "-virtfs synth,mount_tag=tag[,id=id][,readonly]\n",
     QEMU_ARCH_ALL)
 
 STEXI
 
-@item -virtfs local,path=@var{path},mount_tag=@var{mount_tag} ,security_model=@var{security_model}[,writeout=@var{writeout}][,readonly] [,fmode=@var{fmode}][,dmode=@var{dmode}]
-@itemx -virtfs proxy,socket=@var{socket},mount_tag=@var{mount_tag} [,writeout=@var{writeout}][,readonly]
-@itemx -virtfs proxy,sock_fd=@var{sock_fd},mount_tag=@var{mount_tag} [,writeout=@var{writeout}][,readonly]
+@item -virtfs local,path=@var{path},mount_tag=@var{mount_tag} ,security_model=@var{security_model}[,writeout=@var{writeout}][,readonly] [,fmode=@var{fmode}][,dmode=@var{dmode}][,remap_inodes]
+@itemx -virtfs proxy,socket=@var{socket},mount_tag=@var{mount_tag} [,writeout=@var{writeout}][,readonly][,remap_inodes]
+@itemx -virtfs proxy,sock_fd=@var{sock_fd},mount_tag=@var{mount_tag} [,writeout=@var{writeout}][,readonly][,remap_inodes]
 @itemx -virtfs synth,mount_tag=@var{mount_tag}
 @findex -virtfs
 
@@ -1398,6 +1398,19 @@ Specifies the default mode for newly created directories on the host. Works
 only with security models "mapped-xattr" and "mapped-file".
 @item mount_tag=@var{mount_tag}
 Specifies the tag name to be used by the guest to mount this export point.
+@item remap_inodes
+By default virtfs 9p supports only one device per export in order to avoid
+file ID collisions on guest which may otherwise happen because the original
+device IDs from host are not passed and exposed on guest. Instead all files
+of an export shared with virtfs do have the same device id on guest. So two
+files with identical inode numbers but from actually different devices on
+host would otherwise cause a file ID collision and hence potential
+misbehaviours on guest. For that reason it is recommended to create a
+separate virtfs export for each device to be shared with guests. However
+you may also enable "remap_inodes" which allows you to share multiple
+devices with only one export instead, which is achieved by remapping the
+original inode numbers from host to guest in a way that would prevent such
+collisions.
 @end table
 ETEXI
 
diff --git a/vl.c b/vl.c
index 99a56b5556..de9d7b846c 100644
--- a/vl.c
+++ b/vl.c
@@ -3457,6 +3457,9 @@ int main(int argc, char **argv, char **envp)
                 qemu_opt_set_bool(fsdev, "readonly",
                                   qemu_opt_get_bool(opts, "readonly", 0),
                                   &error_abort);
+                qemu_opt_set_bool(fsdev, "remap_inodes",
+                                  qemu_opt_get_bool(opts, "remap_inodes", 0),
+                                  &error_abort);
                 device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
                                           &error_abort);
                 qemu_opt_set(device, "driver", "virtio-9p-pci", &error_abort);
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v5 4/5] 9p: stat_to_qid: implement slow path
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
                   ` (2 preceding siblings ...)
  2019-07-03 11:13 ` [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes' Christian Schoenebeck via Qemu-devel
@ 2019-07-03 11:17 ` Christian Schoenebeck via Qemu-devel
  2019-07-03 11:27 ` [Qemu-devel] [PATCH v5 5/5] 9p: Use variable length suffixes for inode remapping Christian Schoenebeck via Qemu-devel
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-03 11:17 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Greg Kurz, Antonios Motakis

stat_to_qid attempts via qid_path_prefixmap to map unique files (which are
identified by 64 bit inode nr and 32 bit device id) to a 64 QID path value.
However this implementation makes some assumptions about inode number
generation on the host.

If qid_path_prefixmap fails, we still have 48 bits available in the QID
path to fall back to a less memory efficient full mapping.

Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
[CS: - Rebased to master head.
     - Updated hash calls to new xxhash API.
     - Removed unnecessary parantheses in qpf_lookup_func().
     - Removed unnecessary g_malloc0() result checks.
     - Log error message when running out of prefixes in
       qid_path_fullmap().
     - Log error message about potential degraded performance in
       qid_path_prefixmap().
     - Fixed typo in comment. ]
Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------
 hw/9pfs/9p.h |  9 ++++++++
 2 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 39c6c2a894..549e279462 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -579,23 +579,73 @@ static uint32_t qpp_hash(QppEntry e)
     return qemu_xxhash7(e.ino_prefix, e.dev, 0, 0, 0);
 }
 
+static uint32_t qpf_hash(QpfEntry e)
+{
+    return qemu_xxhash7(e.ino, e.dev, 0, 0, 0);
+}
+
 static bool qpp_lookup_func(const void *obj, const void *userp)
 {
     const QppEntry *e1 = obj, *e2 = userp;
     return e1->dev == e2->dev && e1->ino_prefix == e2->ino_prefix;
 }
 
-static void qpp_table_remove(void *p, uint32_t h, void *up)
+static bool qpf_lookup_func(const void *obj, const void *userp)
+{
+    const QpfEntry *e1 = obj, *e2 = userp;
+    return e1->dev == e2->dev && e1->ino == e2->ino;
+}
+
+static void qp_table_remove(void *p, uint32_t h, void *up)
 {
     g_free(p);
 }
 
-static void qpp_table_destroy(struct qht *ht)
+static void qp_table_destroy(struct qht *ht)
 {
-    qht_iter(ht, qpp_table_remove, NULL);
+    qht_iter(ht, qp_table_remove, NULL);
     qht_destroy(ht);
 }
 
+static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
+                            uint64_t *path)
+{
+    QpfEntry lookup = {
+        .dev = stbuf->st_dev,
+        .ino = stbuf->st_ino
+    }, *val;
+    uint32_t hash = qpf_hash(lookup);
+
+    /* most users won't need the fullmap, so init the table lazily */
+    if (!pdu->s->qpf_table.map) {
+        qht_init(&pdu->s->qpf_table, qpf_lookup_func, 1 << 16, QHT_MODE_AUTO_RESIZE);
+    }
+
+    val = qht_lookup(&pdu->s->qpf_table, &lookup, hash);
+
+    if (!val) {
+        if (pdu->s->qp_fullpath_next == 0) {
+            /* no more files can be mapped :'( */
+            error_report_once(
+                "9p: No more prefixes available for remapping inodes from "
+                "host to guest."
+            );
+            return -ENFILE;
+        }
+
+        val = g_malloc0(sizeof(QppEntry));
+        *val = lookup;
+
+        /* new unique inode and device combo */
+        val->path = pdu->s->qp_fullpath_next++;
+        pdu->s->qp_fullpath_next &= QPATH_INO_MASK;
+        qht_insert(&pdu->s->qpf_table, val, hash, NULL);
+    }
+
+    *path = val->path;
+    return 0;
+}
+
 /* stat_to_qid needs to map inode number (64 bits) and device id (32 bits)
  * to a unique QID path (64 bits). To avoid having to map and keep track
  * of up to 2^64 objects, we map only the 16 highest bits of the inode plus
@@ -621,8 +671,7 @@ static int qid_path_prefixmap(V9fsPDU *pdu, const struct stat *stbuf,
         if (pdu->s->qp_prefix_next == 0) {
             /* we ran out of prefixes */
             error_report_once(
-                "9p: No more prefixes available for remapping inodes from "
-                "host to guest."
+                "9p: Potential degraded performance of inode remapping"
             );
             return -ENFILE;
         }
@@ -647,6 +696,10 @@ static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
     if (pdu->s->ctx.export_flags & V9FS_REMAP_INODES) {
         /* map inode+device to qid path (fast path) */
         err = qid_path_prefixmap(pdu, stbuf, &qidp->path);
+        if (err == -ENFILE) {
+            /* fast path didn't work, fall back to full map */
+            err = qid_path_fullmap(pdu, stbuf, &qidp->path);
+        }
         if (err) {
             return err;
         }
@@ -3827,6 +3880,7 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
     /* QID path hash table. 1 entry ought to be enough for anybody ;) */
     qht_init(&s->qpp_table, qpp_lookup_func, 1, QHT_MODE_AUTO_RESIZE);
     s->qp_prefix_next = 1; /* reserve 0 to detect overflow */
+    s->qp_fullpath_next = 1;
 
     s->ctx.fst = &fse->fst;
     fsdev_throttle_init(s->ctx.fst);
@@ -3841,7 +3895,8 @@ out:
         }
         g_free(s->tag);
         g_free(s->ctx.fs_root);
-        qpp_table_destroy(&s->qpp_table);
+        qp_table_destroy(&s->qpp_table);
+        qp_table_destroy(&s->qpf_table);
         v9fs_path_free(&path);
     }
     return rc;
@@ -3854,7 +3909,8 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
     }
     fsdev_throttle_cleanup(s->ctx.fst);
     g_free(s->tag);
-    qpp_table_destroy(&s->qpp_table);
+    qp_table_destroy(&s->qpp_table);
+    qp_table_destroy(&s->qpf_table);
     g_free(s->ctx.fs_root);
 }
 
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index a283b0193e..f044a88a41 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -245,6 +245,13 @@ typedef struct {
     uint16_t qp_prefix;
 } QppEntry;
 
+/* QID path full entry, as above */
+typedef struct {
+    dev_t dev;
+    ino_t ino;
+    uint64_t path;
+} QpfEntry;
+
 struct V9fsState
 {
     QLIST_HEAD(, V9fsPDU) free_list;
@@ -269,7 +276,9 @@ struct V9fsState
     ino_t root_ino;
     dev_t dev_id;
     struct qht qpp_table;
+    struct qht qpf_table;
     uint16_t qp_prefix_next;
+    uint64_t qp_fullpath_next;
 };
 
 /* 9p2000.L open flags */
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v5 5/5] 9p: Use variable length suffixes for inode remapping
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
                   ` (3 preceding siblings ...)
  2019-07-03 11:17 ` [Qemu-devel] [PATCH v5 4/5] 9p: stat_to_qid: implement slow path Christian Schoenebeck via Qemu-devel
@ 2019-07-03 11:27 ` Christian Schoenebeck via Qemu-devel
  2019-07-03 15:46 ` [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions no-reply
  2019-07-03 16:04 ` no-reply
  6 siblings, 0 replies; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-03 11:27 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Greg Kurz, Antonios Motakis

Use variable length suffixes for inode remapping instead of the fixed
16 bit size prefixes before. With this change the inode numbers on guest
will typically be much smaller (e.g. around >2^1 .. >2^7 instead of >2^48
with the previous fixed size inode remapping.

Additionally this solution is more efficient, since inode numbers in
practice can take almost their entire 64 bit range on guest as well, so
there is less likely a need for generating and tracking additional suffixes,
which might also be beneficial for nested virtualization where each level of
virtualization would shift up the inode bits and increase the chance of
expensive remapping actions.

The "Exponential Golomb" algorithm is used as basis for generating the
variable length suffixes. The algorithm has a parameter k which controls the
distribution of bits on increasing indeces (minimum bits at low index vs.
maximum bits at high index). With k=0 the generated suffixes look like:

Index Dec/Bin -> Generated Suffix Bin
1 [1] -> [1] (1 bits)
2 [10] -> [010] (3 bits)
3 [11] -> [110] (3 bits)
4 [100] -> [00100] (5 bits)
5 [101] -> [10100] (5 bits)
6 [110] -> [01100] (5 bits)
7 [111] -> [11100] (5 bits)
8 [1000] -> [0001000] (7 bits)
9 [1001] -> [1001000] (7 bits)
10 [1010] -> [0101000] (7 bits)
11 [1011] -> [1101000] (7 bits)
12 [1100] -> [0011000] (7 bits)
...
65533 [1111111111111101] ->  [1011111111111111000000000000000] (31 bits)
65534 [1111111111111110] ->  [0111111111111111000000000000000] (31 bits)
65535 [1111111111111111] ->  [1111111111111111000000000000000] (31 bits)
Hence minBits=1 maxBits=31

And with k=5 they would look like:

Index Dec/Bin -> Generated Suffix Bin
1 [1] -> [000001] (6 bits)
2 [10] -> [100001] (6 bits)
3 [11] -> [010001] (6 bits)
4 [100] -> [110001] (6 bits)
5 [101] -> [001001] (6 bits)
6 [110] -> [101001] (6 bits)
7 [111] -> [011001] (6 bits)
8 [1000] -> [111001] (6 bits)
9 [1001] -> [000101] (6 bits)
10 [1010] -> [100101] (6 bits)
11 [1011] -> [010101] (6 bits)
12 [1100] -> [110101] (6 bits)
...
65533 [1111111111111101] -> [0011100000000000100000000000] (28 bits)
65534 [1111111111111110] -> [1011100000000000100000000000] (28 bits)
65535 [1111111111111111] -> [0111100000000000100000000000] (28 bits)
Hence minBits=6 maxBits=28

Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
---
 hw/9pfs/9p.c | 247 ++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 hw/9pfs/9p.h |  34 +++++++-
 2 files changed, 251 insertions(+), 30 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 549e279462..5bbd3e2d14 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -26,6 +26,7 @@
 #include "migration/blocker.h"
 #include "sysemu/qtest.h"
 #include "qemu/xxhash.h"
+#include <math.h>
 
 int open_fd_hw;
 int total_open_fd;
@@ -572,6 +573,107 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
                                 P9_STAT_MODE_NAMED_PIPE |   \
                                 P9_STAT_MODE_SOCKET)
 
+/* Mirrors all bits of a byte. So e.g. binary 10100000 would become 00000101. */
+static inline uint8_t mirror8bit(uint8_t byte)
+{
+    return (byte * 0x0202020202ULL & 0x010884422010ULL) % 1023;
+}
+
+/* Same as mirror8bit() just for a 64 bit data type instead for a byte. */
+static inline uint64_t mirror64bit(uint64_t value)
+{
+    return ((uint64_t)mirror8bit( value        & 0xff) << 56) |
+           ((uint64_t)mirror8bit((value >> 8)  & 0xff) << 48) |
+           ((uint64_t)mirror8bit((value >> 16) & 0xff) << 40) |
+           ((uint64_t)mirror8bit((value >> 24) & 0xff) << 32) |
+           ((uint64_t)mirror8bit((value >> 32) & 0xff) << 24) |
+           ((uint64_t)mirror8bit((value >> 40) & 0xff) << 16) |
+           ((uint64_t)mirror8bit((value >> 48) & 0xff) << 8 ) |
+           ((uint64_t)mirror8bit((value >> 56) & 0xff)      ) ;
+}
+
+/** @brief Parameter k for the Exponential Golomb algorihm to be used.
+ *
+ * The smaller this value, the smaller the minimum bit count for the Exp.
+ * Golomb generated affixes will be (at lowest index) however for the
+ * price of having higher maximum bit count of generated affixes (at highest
+ * index). Likewise increasing this parameter yields in smaller maximum bit
+ * count for the price of having higher minimum bit count.
+ *
+ * In practice that means: a good value for k depends on the expected amount
+ * of devices to be exposed by one export. For a small amount of devices k
+ * should be small, for a large amount of devices k might be increased
+ * instead. The default of k=0 should be fine for most users though.
+ *
+ * @b IMPORTANT: In case this ever becomes a runtime parameter; the value of
+ * k should not change as long as guest is still running! Because that would
+ * cause completely different inode numbers to be generated on guest.
+ */
+#define EXP_GOLOMB_K    0
+
+/** @brief Exponential Golomb algorithm for arbitrary k (including k=0).
+ *
+ * The Exponential Golomb algorithm generates @b prefixes (@b not suffixes!)
+ * with growing length and with the mathematical property of being
+ * "prefix-free". The latter means the generated prefixes can be prepended
+ * in front of arbitrary numbers and the resulting concatenated numbers are
+ * guaranteed to be always unique.
+ *
+ * This is a minor adjustment to the original Exp. Golomb algorithm in the
+ * sense that lowest allowed index (@param n) starts with 1, not with zero.
+ *
+ * @param n - natural number (or index) of the prefix to be generated
+ *            (1, 2, 3, ...)
+ * @param k - parameter k of Exp. Golomb algorithm to be used
+ *            (see comment on EXP_GOLOMB_K macro for details about k)
+ */
+static VariLenAffix expGolombEncode(uint64_t n, int k)
+{
+    const uint64_t value = n + (1 << k) - 1;
+    const int bits = (int) log2(value) + 1;
+    return (VariLenAffix) {
+        .type = AffixType_Prefix,
+        .value = value,
+        .bits = bits + MAX((bits - 1 - k), 0)
+    };
+}
+
+/** @brief Converts a suffix into a prefix, or a prefix into a suffix.
+ *
+ * Simply mirror all bits of the affix value, for the purpose to preserve
+ * respectively the mathematical "prefix-free" or "suffix-free" property
+ * after the conversion.
+ *
+ * If a passed prefix is suitable to create unique numbers, then the
+ * returned suffix is suitable to create unique numbers as well (and vice
+ * versa).
+ */
+static VariLenAffix invertAffix(const VariLenAffix* affix)
+{
+    return (VariLenAffix) {
+        .type = (affix->type == AffixType_Suffix) ? AffixType_Prefix : AffixType_Suffix,
+        .value =  mirror64bit(affix->value) >> ((sizeof(affix->value) * 8) - affix->bits),
+        .bits = affix->bits
+    };
+}
+
+/** @brief Generates suffix numbers with "suffix-free" property.
+ *
+ * This is just a wrapper function on top of the Exp. Golomb algorithm.
+ *
+ * Since the Exp. Golomb algorithm generates prefixes, but we need suffixes,
+ * this function converts the Exp. Golomb prefixes into appropriate suffixes
+ * which are still suitable for generating unique numbers.
+ *
+ * @param n - natural number (or index) of the suffix to be generated
+ *            (1, 2, 3, ...)
+ */
+static VariLenAffix affixForIndex(uint64_t index)
+{
+    VariLenAffix prefix;
+    prefix = expGolombEncode(index, EXP_GOLOMB_K);
+    return invertAffix(&prefix); /* convert prefix to suffix */
+}
 
 /* creative abuse of tb_hash_func7, which is based on xxhash */
 static uint32_t qpp_hash(QppEntry e)
@@ -584,13 +686,19 @@ static uint32_t qpf_hash(QpfEntry e)
     return qemu_xxhash7(e.ino, e.dev, 0, 0, 0);
 }
 
-static bool qpp_lookup_func(const void *obj, const void *userp)
+static bool qpd_cmp_func(const void *obj, const void *userp)
+{
+    const QpdEntry *e1 = obj, *e2 = userp;
+    return e1->dev == e2->dev;
+}
+
+static bool qpp_cmp_func(const void *obj, const void *userp)
 {
     const QppEntry *e1 = obj, *e2 = userp;
     return e1->dev == e2->dev && e1->ino_prefix == e2->ino_prefix;
 }
 
-static bool qpf_lookup_func(const void *obj, const void *userp)
+static bool qpf_cmp_func(const void *obj, const void *userp)
 {
     const QpfEntry *e1 = obj, *e2 = userp;
     return e1->dev == e2->dev && e1->ino == e2->ino;
@@ -607,6 +715,54 @@ static void qp_table_destroy(struct qht *ht)
     qht_destroy(ht);
 }
 
+/*
+ * Returns how many (high end) bits of inode numbers of the passed fs
+ * device shall be used (in combination with the device number) to
+ * generate hash values for qpp_table entries.
+ *
+ * This function is required if variable length suffixes are used for inode
+ * number mapping on guest level. Since a device may end up having multiple
+ * entries in qpp_table, each entry most probably with a different suffix
+ * length, we thus need this function in conjunction with qpd_table to
+ * "agree" about a fix amount of bits (per device) to be always used for
+ * generating hash values for the purpose of accessing qpp_table in order
+ * get consistent behaviour when accessing qpp_table.
+ */
+static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t dev)
+{
+    QpdEntry lookup = {
+        .dev = dev
+    }, *val;
+    uint32_t hash = dev;
+    VariLenAffix affix;
+
+    val = qht_lookup(&pdu->s->qpd_table, &lookup, hash);
+    if (!val) {
+        val = g_malloc0(sizeof(QpdEntry));
+        *val = lookup;
+        affix = affixForIndex(pdu->s->qp_affix_next);
+        val->prefix_bits = affix.bits;
+        qht_insert(&pdu->s->qpd_table, val, hash, NULL);
+        pdu->s->qp_ndevices++;
+    }
+    return val->prefix_bits;
+}
+
+/** @brief Slow / full mapping host inode nr -> guest inode nr.
+ *
+ * This function performs a slower and much more costly remapping of an
+ * original file inode number on host to an appropriate different inode
+ * number on guest. For every (dev, inode) combination on host a new
+ * sequential number is generated, cached and exposed as inode number on
+ * guest.
+ *
+ * This is just a "last resort" fallback solution if the much faster/cheaper
+ * qid_path_prefixmap() failed. In practice this slow / full mapping is not
+ * expected ever to be used at all though.
+ *
+ * @see qid_path_prefixmap() for details
+ *
+ */
 static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
                             uint64_t *path)
 {
@@ -615,11 +771,7 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
         .ino = stbuf->st_ino
     }, *val;
     uint32_t hash = qpf_hash(lookup);
-
-    /* most users won't need the fullmap, so init the table lazily */
-    if (!pdu->s->qpf_table.map) {
-        qht_init(&pdu->s->qpf_table, qpf_lookup_func, 1 << 16, QHT_MODE_AUTO_RESIZE);
-    }
+    VariLenAffix affix;
 
     val = qht_lookup(&pdu->s->qpf_table, &lookup, hash);
 
@@ -637,8 +789,11 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
         *val = lookup;
 
         /* new unique inode and device combo */
-        val->path = pdu->s->qp_fullpath_next++;
-        pdu->s->qp_fullpath_next &= QPATH_INO_MASK;
+        affix = affixForIndex(
+            1ULL << (sizeof(pdu->s->qp_affix_next) * 8)
+        );
+        val->path = (pdu->s->qp_fullpath_next++ << affix.bits) | affix.value;
+        pdu->s->qp_fullpath_next &= ((1ULL << (64 - affix.bits)) - 1);
         qht_insert(&pdu->s->qpf_table, val, hash, NULL);
     }
 
@@ -646,30 +801,59 @@ static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
     return 0;
 }
 
-/* stat_to_qid needs to map inode number (64 bits) and device id (32 bits)
- * to a unique QID path (64 bits). To avoid having to map and keep track
- * of up to 2^64 objects, we map only the 16 highest bits of the inode plus
- * the device id to the 16 highest bits of the QID path. The 48 lowest bits
- * of the QID path equal to the lowest bits of the inode number.
+/** @brief Quick mapping host inode nr -> guest inode nr.
  *
- * This takes advantage of the fact that inode number are usually not
- * random but allocated sequentially, so we have fewer items to keep
- * track of.
+ * This function performs quick remapping of an original file inode number
+ * on host to an appropriate different inode number on guest. This remapping
+ * of inodes is required to avoid inode nr collisions on guest which would
+ * happen if the 9p export contains more than 1 exported file system (or
+ * more than 1 file system data set), because unlike on host level where the
+ * files would have different device nrs, all files exported by 9p would
+ * share the same device nr on guest (the device nr of the virtual 9p device
+ * that is).
+ *
+ * Inode remapping is performed by chopping off high end bits of the original
+ * inode number from host, shifting the result upwards and then assigning a
+ * generated suffix number for the low end bits, where the same suffix number
+ * will be shared by all inodes with the same device id AND the same high end
+ * bits that have been chopped off. That approach utilizes the fact that inode
+ * numbers very likely share the same high end bits (i.e. due to their common
+ * sequential generation by file systems) and hence we only have to generate
+ * and track a very limited amount of suffixes in practice due to that.
+ *
+ * We generate variable size suffixes for that purpose. The 1st generated
+ * suffix will only have 1 bit and hence we only need to chop off 1 bit from
+ * the original inode number. The subsequent suffixes being generated will
+ * grow in (bit) size subsequently, i.e. the 2nd and 3rd suffix being
+ * generated will have 3 bits and hence we have to chop off 3 bits from their
+ * original inodes, and so on. That approach of using variable length suffixes
+ * (i.e. over fixed size ones) utilizes the fact that in practice only a very
+ * limited amount of devices are shared by the same export (e.g. typically
+ * less than 2 dozen devices per 9p export), so in practice we need to chop
+ * off less bits than with fixed size prefixes and yet are flexible to add
+ * new devices at runtime below host's export directory at any time without
+ * having to reboot guest nor requiring to reconfigure guest for that. And due
+ * to the very limited amount of original high end bits that we chop off that
+ * way, the total amount of suffixes we need to generate is less than by using
+ * fixed size prefixes and hence it also improves performance of the inode
+ * remapping algorithm, and finally has the nice side effect that the inode
+ * numbers on guest will be much smaller & human friendly. ;-)
  */
 static int qid_path_prefixmap(V9fsPDU *pdu, const struct stat *stbuf,
                                 uint64_t *path)
 {
+    const int ino_hash_bits = qid_inode_prefix_hash_bits(pdu, stbuf->st_dev);
     QppEntry lookup = {
         .dev = stbuf->st_dev,
-        .ino_prefix = (uint16_t) (stbuf->st_ino >> 48)
+        .ino_prefix = (uint16_t) (stbuf->st_ino >> (64-ino_hash_bits))
     }, *val;
     uint32_t hash = qpp_hash(lookup);
 
     val = qht_lookup(&pdu->s->qpp_table, &lookup, hash);
 
     if (!val) {
-        if (pdu->s->qp_prefix_next == 0) {
-            /* we ran out of prefixes */
+        if (pdu->s->qp_affix_next == 0) {
+            /* we ran out of affixes */
             error_report_once(
                 "9p: Potential degraded performance of inode remapping"
             );
@@ -679,12 +863,13 @@ static int qid_path_prefixmap(V9fsPDU *pdu, const struct stat *stbuf,
         val = g_malloc0(sizeof(QppEntry));
         *val = lookup;
 
-        /* new unique inode prefix and device combo */
-        val->qp_prefix = pdu->s->qp_prefix_next++;
+        /* new unique inode affix and device combo */
+        val->qp_affix_index = pdu->s->qp_affix_next++;
+        val->qp_affix = affixForIndex(val->qp_affix_index);
         qht_insert(&pdu->s->qpp_table, val, hash, NULL);
     }
-
-    *path = ((uint64_t)val->qp_prefix << 48) | (stbuf->st_ino & QPATH_INO_MASK);
+    /* assuming generated affix to be suffix type, not prefix */
+    *path = (stbuf->st_ino << val->qp_affix.bits) | val->qp_affix.value;
     return 0;
 }
 
@@ -3877,9 +4062,15 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
     s->root_ino = stat.st_ino;
     s->dev_id = stat.st_dev;
 
-    /* QID path hash table. 1 entry ought to be enough for anybody ;) */
-    qht_init(&s->qpp_table, qpp_lookup_func, 1, QHT_MODE_AUTO_RESIZE);
-    s->qp_prefix_next = 1; /* reserve 0 to detect overflow */
+    /* init inode remapping : */
+    /* hash table for variable length inode suffixes */
+    qht_init(&s->qpd_table, qpd_cmp_func, 1, QHT_MODE_AUTO_RESIZE);
+    /* hash table for slow/full inode remapping (most users won't need it) */
+    qht_init(&s->qpf_table, qpf_cmp_func, 1 << 16, QHT_MODE_AUTO_RESIZE);
+    /* hash table for quick inode remapping */
+    qht_init(&s->qpp_table, qpp_cmp_func, 1, QHT_MODE_AUTO_RESIZE);
+    s->qp_ndevices = 0;
+    s->qp_affix_next = 1; /* reserve 0 to detect overflow */
     s->qp_fullpath_next = 1;
 
     s->ctx.fst = &fse->fst;
@@ -3895,6 +4086,7 @@ out:
         }
         g_free(s->tag);
         g_free(s->ctx.fs_root);
+        qp_table_destroy(&s->qpd_table);
         qp_table_destroy(&s->qpp_table);
         qp_table_destroy(&s->qpf_table);
         v9fs_path_free(&path);
@@ -3909,6 +4101,7 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
     }
     fsdev_throttle_cleanup(s->ctx.fst);
     g_free(s->tag);
+    qp_table_destroy(&s->qpd_table);
     qp_table_destroy(&s->qpp_table);
     qp_table_destroy(&s->qpf_table);
     g_free(s->ctx.fs_root);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index f044a88a41..597b2c7222 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -236,13 +236,39 @@ struct V9fsFidState
     V9fsFidState *rclm_lst;
 };
 
-#define QPATH_INO_MASK        ((1ULL << 48) - 1)
+typedef enum AffixType_t {
+    AffixType_Prefix,
+    AffixType_Suffix, /* A.k.a. postfix. */
+} AffixType_t;
+
+/** @brief Unique affix of variable length.
+ *
+ * An affix is (currently) either a suffix or a prefix, which is either
+ * going to be prepended (prefix) or appended (suffix) with some other
+ * number for the goal to generate unique numbers. Accordingly the
+ * suffixes (or prefixes) we generate @b must all have the mathematical
+ * property of being suffix-free (or prefix-free in case of prefixes)
+ * so that no matter what number we concatenate the affix with, that we
+ * always reliably get unique numbers as result after concatenation.
+ */
+typedef struct VariLenAffix {
+    AffixType_t type; /* Whether this affix is a suffix or a prefix. */
+    uint64_t value; /* Actual numerical value of this affix. */
+    int bits; /* Lenght of the affix, that is how many (of the lowest) bits of @c value must be used for appending/prepending this affix to its final resulting, unique number. */
+} VariLenAffix;
+
+/* See qid_inode_prefix_hash_bits(). */
+typedef struct {
+    dev_t dev; /* FS device on host. */
+    int prefix_bits; /* How many (high) bits of the original inode number shall be used for hashing. */
+} QpdEntry;
 
 /* QID path prefix entry, see stat_to_qid */
 typedef struct {
     dev_t dev;
     uint16_t ino_prefix;
-    uint16_t qp_prefix;
+    uint32_t qp_affix_index;
+    VariLenAffix qp_affix;
 } QppEntry;
 
 /* QID path full entry, as above */
@@ -275,9 +301,11 @@ struct V9fsState
     V9fsQID root_qid;
     ino_t root_ino;
     dev_t dev_id;
+    struct qht qpd_table;
     struct qht qpp_table;
     struct qht qpf_table;
-    uint16_t qp_prefix_next;
+    uint64_t qp_ndevices; /* Amount of entries in qpd_table. */
+    uint16_t qp_affix_next;
     uint64_t qp_fullpath_next;
 };
 
-- 
2.11.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions
@ 2019-07-03 11:44 Christian Schoenebeck via Qemu-devel
  2019-07-03 10:55 ` [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path Christian Schoenebeck via Qemu-devel
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-03 11:44 UTC (permalink / raw)
  To: qemu-devel; +Cc: Daniel P. Berrangé, Greg Kurz, Antonios Motakis

This is v5 of a proposed patch set for fixing file ID collisions with 9pfs.

v4->v5:

  All Patches:

  * Added details to individual commit logs of what has been changed
    exactly by me on top of Antonios' original 4 patches.

  Patch 1:

  * Fixed format specifiers in hw/9pfs/trace-events.

  Patch 2:

  * Fixed typo in commit log.

  * Assign dev_id to export root's device already in
    v9fs_device_realize_common(), not postponed in stat_to_qid().

  * Return -ENODEV instead of -ENOSYS in stat_to_qid().

  Patch 3:

  * Added missing manual parts for new virtfs option 'remap_inodes' in
    qemu-options.hx.

  * Capture root_ino in v9fs_device_realize_common() as well, not just the
    device id.

  * Added function dirent_to_qid().

  * Fixed v9fs_do_readdir() having exposed info outside export root with
    '..' entry (no matter if inode remapping was on or not).

  * Fixed v9fs_do_readdir() not having remapped inodes.

  * Fixed definition of QPATH_INO_MASK.

  * Log error message when running out of prefixes in qid_path_prefixmap().

  * Adjusted changes in stat_to_qid() to qemu code style guidelines.

  Patch 4:

  * Log error message when running out of prefixes in qid_path_fullmap().

  * Log error message about potential degraded performance in
    qid_path_prefixmap() (that is when qid_path_fullmap() will start to
    kick in next).

  * Fixed typo in code comment.

  Patch 5:

  * Dropped fixed (16 bit) size prefix code and hence removed usage of
    P9_VARI_LENGTH_INODE_SUFFIXES macro checks all over the place.

  * Dropped function expGolombEncodeK0(uint64_t n) which was optimized for
    the expected default value of k=0; instead always use the generalized
    function expGolombEncode(uint64_t n, int k) instead now.

  * Adjusted changes in hw/9pfs/9p.c to qemu code style guidelines.

  * Adjusted functions' API comments in hw/9pfs/9p.c.

v3->v4:

  * Rebased to latest git master head.

  * Splitted Antonios' patch set to its original 4 individual patches.
    (was merged previously as only 1 patch).

  * Addressed discussed issues directly on Antonios' patches
    (was a separate patch before).

  * Added virtfs command line option "remap_inodes": Unless this option
    is not enabled, no inode remapping is performed at all, the user
    just gets an error message when trying to use more than 1 device
    per export.

  * Dropped persistency feature of QIDs beyond reboots.

  * Dropped disputed "vii" feature.

Christian Schoenebeck (5):
  9p: unsigned type for type, version, path
  9p: Treat multiple devices on one export as an error
  9p: Added virtfs option 'remap_inodes'
  9p: stat_to_qid: implement slow path
  9p: Use variable length suffixes for inode remapping

 fsdev/9p-marshal.h      |   6 +-
 fsdev/file-op-9p.h      |   1 +
 fsdev/qemu-fsdev-opts.c |   7 +-
 fsdev/qemu-fsdev.c      |   6 +
 hw/9pfs/9p.c            | 508 +++++++++++++++++++++++++++++++++++++++++++++---
 hw/9pfs/9p.h            |  51 +++++
 hw/9pfs/trace-events    |  14 +-
 qemu-options.hx         |  25 ++-
 vl.c                    |   3 +
 9 files changed, 573 insertions(+), 48 deletions(-)

-- 
2.11.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
                   ` (4 preceding siblings ...)
  2019-07-03 11:27 ` [Qemu-devel] [PATCH v5 5/5] 9p: Use variable length suffixes for inode remapping Christian Schoenebeck via Qemu-devel
@ 2019-07-03 15:46 ` no-reply
  2019-07-03 16:04 ` no-reply
  6 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2019-07-03 15:46 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, qemu-devel, antonios.motakis, groug

Patchew URL: https://patchew.org/QEMU/cover.1562154272.git.qemu_oss@crudebyte.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions
Message-id: cover.1562154272.git.qemu_oss@crudebyte.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Switched to a new branch 'test'
0894392 9p: Use variable length suffixes for inode remapping
85bd3cf 9p: stat_to_qid: implement slow path
55ea63d 9p: Added virtfs option 'remap_inodes'
86cbf21 9p: Treat multiple devices on one export as an error
d864b5a 9p: unsigned type for type, version, path

=== OUTPUT BEGIN ===
1/5 Checking commit d864b5ad19ff (9p: unsigned type for type, version, path)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 68 lines checked

Patch 1/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

2/5 Checking commit 86cbf21432ea (9p: Treat multiple devices on one export as an error)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>

total: 1 errors, 0 warnings, 175 lines checked

Patch 2/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

3/5 Checking commit 55ea63dbafc2 (9p: Added virtfs option 'remap_inodes')
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>

WARNING: Block comments use a leading /* on a separate line
#144: FILE: hw/9pfs/9p.c:599:
+/* stat_to_qid needs to map inode number (64 bits) and device id (32 bits)

total: 1 errors, 1 warnings, 382 lines checked

Patch 3/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

4/5 Checking commit 85bd3cf1f51f (9p: stat_to_qid: implement slow path)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>

WARNING: line over 80 characters
#78: FILE: hw/9pfs/9p.c:621:
+        qht_init(&pdu->s->qpf_table, qpf_lookup_func, 1 << 16, QHT_MODE_AUTO_RESIZE);

total: 1 errors, 1 warnings, 142 lines checked

Patch 4/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/5 Checking commit 089439298922 (9p: Use variable length suffixes for inode remapping)
ERROR: Author email address is mangled by the mailing list
#2: 
Author: Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org>

ERROR: space prohibited after that open parenthesis '('
#92: FILE: hw/9pfs/9p.c:585:
+    return ((uint64_t)mirror8bit( value        & 0xff) << 56) |

ERROR: space prohibited before that close parenthesis ')'
#98: FILE: hw/9pfs/9p.c:591:
+           ((uint64_t)mirror8bit((value >> 48) & 0xff) << 8 ) |

ERROR: space prohibited before that close parenthesis ')'
#99: FILE: hw/9pfs/9p.c:592:
+           ((uint64_t)mirror8bit((value >> 56) & 0xff)      ) ;

WARNING: Block comments use a leading /* on a separate line
#102: FILE: hw/9pfs/9p.c:595:
+/** @brief Parameter k for the Exponential Golomb algorihm to be used.

WARNING: Block comments use a leading /* on a separate line
#121: FILE: hw/9pfs/9p.c:614:
+/** @brief Exponential Golomb algorithm for arbitrary k (including k=0).

WARNING: Block comments use a leading /* on a separate line
#148: FILE: hw/9pfs/9p.c:641:
+/** @brief Converts a suffix into a prefix, or a prefix into a suffix.

ERROR: "foo* bar" should be "foo *bar"
#158: FILE: hw/9pfs/9p.c:651:
+static VariLenAffix invertAffix(const VariLenAffix* affix)

WARNING: line over 80 characters
#161: FILE: hw/9pfs/9p.c:654:
+        .type = (affix->type == AffixType_Suffix) ? AffixType_Prefix : AffixType_Suffix,

WARNING: line over 80 characters
#162: FILE: hw/9pfs/9p.c:655:
+        .value =  mirror64bit(affix->value) >> ((sizeof(affix->value) * 8) - affix->bits),

WARNING: Block comments use a leading /* on a separate line
#167: FILE: hw/9pfs/9p.c:660:
+/** @brief Generates suffix numbers with "suffix-free" property.

WARNING: Block comments use a leading /* on a separate line
#246: FILE: hw/9pfs/9p.c:751:
+/** @brief Slow / full mapping host inode nr -> guest inode nr.

WARNING: Block comments use a leading /* on a separate line
#300: FILE: hw/9pfs/9p.c:804:
+/** @brief Quick mapping host inode nr -> guest inode nr.

ERROR: spaces required around that '-' (ctx:VxV)
#348: FILE: hw/9pfs/9p.c:848:
+        .ino_prefix = (uint16_t) (stbuf->st_ino >> (64-ino_hash_bits))
                                                       ^

WARNING: Block comments use a leading /* on a separate line
#429: FILE: hw/9pfs/9p.h:244:
+/** @brief Unique affix of variable length.

ERROR: line over 90 characters
#442: FILE: hw/9pfs/9p.h:257:
+    int bits; /* Lenght of the affix, that is how many (of the lowest) bits of @c value must be used for appending/prepending this affix to its final resulting, unique number. */

ERROR: line over 90 characters
#448: FILE: hw/9pfs/9p.h:263:
+    int prefix_bits; /* How many (high) bits of the original inode number shall be used for hashing. */

total: 8 errors, 9 warnings, 386 lines checked

Patch 5/5 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/cover.1562154272.git.qemu_oss@crudebyte.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions
  2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
                   ` (5 preceding siblings ...)
  2019-07-03 15:46 ` [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions no-reply
@ 2019-07-03 16:04 ` no-reply
  6 siblings, 0 replies; 13+ messages in thread
From: no-reply @ 2019-07-03 16:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: berrange, qemu-devel, antonios.motakis, groug

Patchew URL: https://patchew.org/QEMU/cover.1562154272.git.qemu_oss@crudebyte.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

PASS 1 fdc-test /x86_64/fdc/cmos
PASS 2 fdc-test /x86_64/fdc/no_media_on_start
PASS 3 fdc-test /x86_64/fdc/read_without_media
==7929==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 fdc-test /x86_64/fdc/media_change
PASS 1 check-qlit /qlit/equal_qobject
PASS 2 check-qlit /qlit/qobject_from_qlit
---
PASS 32 test-opts-visitor /visitor/opts/range/beyond
PASS 33 test-opts-visitor /visitor/opts/dict/unvisited
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-coroutine -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-coroutine" 
==7973==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==7973==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe7eb6d000; bottom 0x7ff77dcf8000; size: 0x000700e75000 (30079930368)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 1 test-coroutine /basic/no-dangling-access
---
PASS 11 test-aio /aio/event/wait
PASS 12 test-aio /aio/event/flush
PASS 13 test-aio /aio/event/wait/no-flush-cb
==7988==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-aio /aio/timer/schedule
PASS 15 test-aio /aio/coroutine/queue-chaining
PASS 16 test-aio /aio-gsource/flush
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-aio-multithread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-aio-multithread" 
PASS 13 fdc-test /x86_64/fdc/fuzz-registers
PASS 1 test-aio-multithread /aio/multi/lifecycle
==7994==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ide-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ide-test" 
PASS 2 test-aio-multithread /aio/multi/schedule
==8012==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ide-test /x86_64/ide/identify
PASS 3 test-aio-multithread /aio/multi/mutex/contended
==8023==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 ide-test /x86_64/ide/flush
==8034==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 ide-test /x86_64/ide/bmdma/simple_rw
==8040==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 test-aio-multithread /aio/multi/mutex/handoff
PASS 4 ide-test /x86_64/ide/bmdma/trim
==8051==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 test-aio-multithread /aio/multi/mutex/mcs
PASS 5 ide-test /x86_64/ide/bmdma/short_prdt
==8062==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 test-aio-multithread /aio/multi/mutex/pthread
PASS 6 ide-test /x86_64/ide/bmdma/one_sector_short_prdt
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-throttle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-throttle" 
==8071==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-throttle /throttle/leak_bucket
PASS 2 test-throttle /throttle/compute_wait
PASS 3 test-throttle /throttle/init
---
PASS 13 test-throttle /throttle/config/ranges
PASS 14 test-throttle /throttle/config/max
PASS 15 test-throttle /throttle/config/iops_size
==8069==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-thread-pool -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-thread-pool" 
PASS 1 test-thread-pool /thread-pool/submit
PASS 2 test-thread-pool /thread-pool/submit-aio
==8080==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-thread-pool /thread-pool/submit-co
PASS 4 test-thread-pool /thread-pool/submit-many
PASS 7 ide-test /x86_64/ide/bmdma/long_prdt
==8148==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8148==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff7b0e3000; bottom 0x7fbbb7526000; size: 0x0043c3bbd000 (291046674432)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 8 ide-test /x86_64/ide/bmdma/no_busmaster
PASS 5 test-thread-pool /thread-pool/cancel
PASS 9 ide-test /x86_64/ide/flush/nodev
==8160==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 ide-test /x86_64/ide/flush/empty_drive
PASS 6 test-thread-pool /thread-pool/cancel-async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-hbitmap -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-hbitmap" 
==8165==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-hbitmap /hbitmap/granularity
PASS 2 test-hbitmap /hbitmap/size/0
PASS 3 test-hbitmap /hbitmap/size/unaligned
---
PASS 10 test-hbitmap /hbitmap/set/all
PASS 11 test-hbitmap /hbitmap/set/one
PASS 12 test-hbitmap /hbitmap/set/two-elem
==8176==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 test-hbitmap /hbitmap/set/general
PASS 14 test-hbitmap /hbitmap/set/twice
PASS 15 test-hbitmap /hbitmap/set/overlap
PASS 16 test-hbitmap /hbitmap/reset/empty
PASS 12 ide-test /x86_64/ide/flush/retry_isa
PASS 17 test-hbitmap /hbitmap/reset/general
==8182==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 test-hbitmap /hbitmap/reset/all
PASS 19 test-hbitmap /hbitmap/truncate/nop
PASS 20 test-hbitmap /hbitmap/truncate/grow/negligible
---
PASS 29 test-hbitmap /hbitmap/truncate/shrink/large
PASS 30 test-hbitmap /hbitmap/meta/zero
PASS 13 ide-test /x86_64/ide/cdrom/pio
==8188==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 ide-test /x86_64/ide/cdrom/pio_large
==8194==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 31 test-hbitmap /hbitmap/meta/one
PASS 32 test-hbitmap /hbitmap/meta/byte
PASS 33 test-hbitmap /hbitmap/meta/word
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/ahci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="ahci-test" 
PASS 34 test-hbitmap /hbitmap/meta/sector
PASS 35 test-hbitmap /hbitmap/serialize/align
==8208==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 ahci-test /x86_64/ahci/sanity
==8214==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 36 test-hbitmap /hbitmap/serialize/basic
PASS 37 test-hbitmap /hbitmap/serialize/part
PASS 38 test-hbitmap /hbitmap/serialize/zeroes
---
PASS 42 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_1
PASS 43 test-hbitmap /hbitmap/next_dirty_area/next_dirty_area_4
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-drain -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-drain" 
==8223==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-drain /bdrv-drain/nested
PASS 2 test-bdrv-drain /bdrv-drain/multiparent
PASS 3 test-bdrv-drain /bdrv-drain/set_aio_context
---
PASS 21 test-bdrv-drain /bdrv-drain/blockjob/drain_all
PASS 22 test-bdrv-drain /bdrv-drain/blockjob/drain
PASS 23 test-bdrv-drain /bdrv-drain/blockjob/drain_subtree
==8220==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 24 test-bdrv-drain /bdrv-drain/blockjob/error/drain_all
PASS 25 test-bdrv-drain /bdrv-drain/blockjob/error/drain
PASS 26 test-bdrv-drain /bdrv-drain/blockjob/error/drain_subtree
---
PASS 39 test-bdrv-drain /bdrv-drain/attach/drain
PASS 3 ahci-test /x86_64/ahci/pci_enable
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bdrv-graph-mod -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bdrv-graph-mod" 
==8269==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-bdrv-graph-mod /bdrv-graph-mod/update-perm-tree
PASS 2 test-bdrv-graph-mod /bdrv-graph-mod/should-update-child
==8267==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-blockjob -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob" 
==8279==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob /blockjob/ids
PASS 2 test-blockjob /blockjob/cancel/created
PASS 3 test-blockjob /blockjob/cancel/running
---
PASS 8 test-blockjob /blockjob/cancel/concluded
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-blockjob-txn -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-blockjob-txn" 
PASS 4 ahci-test /x86_64/ahci/hba_spec
==8284==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-blockjob-txn /single/success
PASS 2 test-blockjob-txn /single/failure
PASS 3 test-blockjob-txn /single/cancel
---
PASS 5 test-blockjob-txn /pair/failure
PASS 6 test-blockjob-txn /pair/cancel
PASS 7 test-blockjob-txn /pair/fail-cancel-race
==8286==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-block-backend -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-backend" 
==8294==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-backend /block-backend/drain_aio_error
PASS 2 test-block-backend /block-backend/drain_all_aio_error
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-block-iothread -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-block-iothread" 
PASS 5 ahci-test /x86_64/ahci/hba_enable
==8300==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-block-iothread /sync-op/pread
PASS 2 test-block-iothread /sync-op/pwrite
PASS 3 test-block-iothread /sync-op/load_vmstate
---
PASS 14 test-block-iothread /propagate/basic
PASS 15 test-block-iothread /propagate/diamond
PASS 16 test-block-iothread /propagate/mirror
==8302==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-image-locking -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-image-locking" 
==8326==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-image-locking /image-locking/basic
PASS 2 test-image-locking /image-locking/set-perm-abort
PASS 6 ahci-test /x86_64/ahci/identify
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-x86-cpuid -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid" 
PASS 1 test-x86-cpuid /cpuid/topology/basic
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-xbzrle -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-xbzrle" 
==8331==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-xbzrle /xbzrle/uleb
PASS 2 test-xbzrle /xbzrle/encode_decode_zero
PASS 3 test-xbzrle /xbzrle/encode_decode_unchanged
---
PASS 7 ahci-test /x86_64/ahci/max
PASS 6 test-xbzrle /xbzrle/encode_decode
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-vmstate -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-vmstate" 
==8344==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-vmstate /vmstate/tmp_struct
PASS 2 test-vmstate /vmstate/simple/primitive
PASS 3 test-vmstate /vmstate/simple/array
---
PASS 10 test-int128 /int128/int128_rshift
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/rcutorture -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="rcutorture" 
PASS 8 ahci-test /x86_64/ahci/reset
==8383==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 rcutorture /rcu/torture/1reader
==8383==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcdaa54000; bottom 0x7efc703fe000; size: 0x01006a656000 (1101296656384)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 9 ahci-test /x86_64/ahci/io/pio/lba28/simple/zero
==8405==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 rcutorture /rcu/torture/10readers
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-list -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-list" 
==8405==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe2c241000; bottom 0x7f06913fe000; size: 0x00f79ae43000 (1063455567872)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 10 ahci-test /x86_64/ahci/io/pio/lba28/simple/low
PASS 1 test-rcu-list /rcu/qlist/single-threaded
==8418==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8418==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffefeacb000; bottom 0x7f70301fe000; size: 0x008ece8cd000 (613350690816)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 11 ahci-test /x86_64/ahci/io/pio/lba28/simple/high
PASS 2 test-rcu-list /rcu/qlist/short-few
==8430==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8430==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff83dae000; bottom 0x7f12ea9fe000; size: 0x00ec993b0000 (1016183062528)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 12 ahci-test /x86_64/ahci/io/pio/lba28/double/zero
==8457==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-rcu-list /rcu/qlist/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-simpleq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-simpleq" 
==8457==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe1592e000; bottom 0x7f3ed55fe000; size: 0x00bf40330000 (821415837696)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 13 ahci-test /x86_64/ahci/io/pio/lba28/double/low
==8470==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-rcu-simpleq /rcu/qsimpleq/single-threaded
==8470==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffeec318000; bottom 0x7f0e379fe000; size: 0x00f0b491a000 (1033821593600)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 14 ahci-test /x86_64/ahci/io/pio/lba28/double/high
==8482==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-rcu-simpleq /rcu/qsimpleq/short-few
==8482==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff5e64f000; bottom 0x7fd9681fe000; size: 0x0025f6451000 (163045511168)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 15 ahci-test /x86_64/ahci/io/pio/lba28/long/zero
==8509==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8509==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffc037e1000; bottom 0x7fb93db24000; size: 0x0042c5cbd000 (286786310144)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 3 test-rcu-simpleq /rcu/qsimpleq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-rcu-tailq -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-rcu-tailq" 
PASS 16 ahci-test /x86_64/ahci/io/pio/lba28/long/low
==8522==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-rcu-tailq /rcu/qtailq/single-threaded
==8522==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe8ad18000; bottom 0x7f1939b7c000; size: 0x00e55119c000 (984908152832)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 17 ahci-test /x86_64/ahci/io/pio/lba28/long/high
PASS 2 test-rcu-tailq /rcu/qtailq/short-few
==8534==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 18 ahci-test /x86_64/ahci/io/pio/lba28/short/zero
==8561==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-rcu-tailq /rcu/qtailq/long-many
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qdist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qdist" 
PASS 19 ahci-test /x86_64/ahci/io/pio/lba28/short/low
---
PASS 7 test-qdist /qdist/binning/expand
PASS 8 test-qdist /qdist/binning/shrink
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qht -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht" 
==8572==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 20 ahci-test /x86_64/ahci/io/pio/lba28/short/high
==8582==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8582==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdd4444000; bottom 0x7fbf209fe000; size: 0x003eb3a46000 (269301866496)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 21 ahci-test /x86_64/ahci/io/pio/lba48/simple/zero
==8588==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8588==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffebc4d2000; bottom 0x7fe3e3ffe000; size: 0x001ad84d4000 (115298091008)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 22 ahci-test /x86_64/ahci/io/pio/lba48/simple/low
==8594==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8594==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fff7a956000; bottom 0x7f7d5d1fe000; size: 0x00821d758000 (558839988224)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 23 ahci-test /x86_64/ahci/io/pio/lba48/simple/high
==8600==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8600==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffda828000; bottom 0x7f9bd07fe000; size: 0x00640a02a000 (429664673792)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 24 ahci-test /x86_64/ahci/io/pio/lba48/double/zero
==8606==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8606==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffd0524b000; bottom 0x7f648c5fe000; size: 0x009878c4d000 (654861193216)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 25 ahci-test /x86_64/ahci/io/pio/lba48/double/low
==8612==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8612==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffffb586000; bottom 0x7fae66ffe000; size: 0x005194588000 (350381178880)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 26 ahci-test /x86_64/ahci/io/pio/lba48/double/high
==8618==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8618==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7fffb0067000; bottom 0x7fa4e717c000; size: 0x005ac8eeb000 (389918142464)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 27 ahci-test /x86_64/ahci/io/pio/lba48/long/zero
==8624==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8624==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffdd36ed000; bottom 0x7fbd77dfe000; size: 0x00405b8ef000 (276414001152)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 28 ahci-test /x86_64/ahci/io/pio/lba48/long/low
==8630==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==8630==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe84918000; bottom 0x7f6f7977c000; size: 0x008f0b19c000 (614366560256)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 29 ahci-test /x86_64/ahci/io/pio/lba48/long/high
==8636==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 30 ahci-test /x86_64/ahci/io/pio/lba48/short/zero
PASS 1 test-qht /qht/mode/default
==8642==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-qht /qht/mode/resize
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qht-par -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qht-par" 
PASS 31 ahci-test /x86_64/ahci/io/pio/lba48/short/low
==8658==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-qht-par /qht/parallel/2threads-0%updates-1s
PASS 32 ahci-test /x86_64/ahci/io/pio/lba48/short/high
==8671==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 test-qht-par /qht/parallel/2threads-20%updates-1s
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bitops -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bitops" 
PASS 1 test-bitops /bitops/sextract32
---
PASS 3 test-qdev-global-props /qdev/properties/dynamic/global
PASS 4 test-qdev-global-props /qdev/properties/global/subclass
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/check-qom-interface -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="check-qom-interface" 
==8687==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 check-qom-interface /qom/interface/direct_impl
PASS 2 check-qom-interface /qom/interface/intermediate_impl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/check-qom-proplist -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="check-qom-proplist" 
---
PASS 9 test-keyval /keyval/visit/alternate
PASS 10 test-keyval /keyval/visit/any
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-write-threshold -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-write-threshold" 
==8716==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-write-threshold /write-threshold/not-set-on-init
PASS 2 test-write-threshold /write-threshold/set-get
PASS 3 test-write-threshold /write-threshold/multi-set-get
---
PASS 27 test-crypto-cipher /crypto/cipher/null-iv
PASS 28 test-crypto-cipher /crypto/cipher/short-plaintext
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-secret -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-secret" 
==8741==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-secret /crypto/secret/direct
PASS 2 test-crypto-secret /crypto/secret/indirect/good
PASS 3 test-crypto-secret /crypto/secret/indirect/badfile
---
PASS 16 test-crypto-secret /crypto/secret/crypt/badiv
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-tlscredsx509 -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlscredsx509" 
PASS 36 ahci-test /x86_64/ahci/io/dma/lba28/simple/low
==8761==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 37 ahci-test /x86_64/ahci/io/dma/lba28/simple/high
PASS 1 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectserver
PASS 2 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/perfectclient
PASS 3 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca1
==8767==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca2
PASS 38 ahci-test /x86_64/ahci/io/dma/lba28/double/zero
PASS 5 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodca3
PASS 6 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca1
PASS 7 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca2
PASS 8 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badca3
==8773==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver1
PASS 39 ahci-test /x86_64/ahci/io/dma/lba28/double/low
PASS 10 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver2
PASS 11 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver3
PASS 12 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver4
PASS 13 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver5
==8779==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver6
PASS 15 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/goodserver7
PASS 16 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/badserver1
---
PASS 33 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive2
PASS 34 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/inactive3
PASS 40 ahci-test /x86_64/ahci/io/dma/lba28/double/high
==8785==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 35 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain1
PASS 36 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/chain2
PASS 37 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingca
---
PASS 39 test-crypto-tlscredsx509 /qcrypto/tlscredsx509/missingclient
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-tlssession -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-tlssession" 
PASS 41 ahci-test /x86_64/ahci/io/dma/lba28/long/zero
==8796==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-tlssession /qcrypto/tlssession/psk
PASS 2 test-crypto-tlssession /qcrypto/tlssession/basicca
PASS 42 ahci-test /x86_64/ahci/io/dma/lba28/long/low
==8802==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 test-crypto-tlssession /qcrypto/tlssession/differentca
PASS 4 test-crypto-tlssession /qcrypto/tlssession/altname1
PASS 43 ahci-test /x86_64/ahci/io/dma/lba28/long/high
==8808==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 44 ahci-test /x86_64/ahci/io/dma/lba28/short/zero
PASS 5 test-crypto-tlssession /qcrypto/tlssession/altname2
==8814==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 45 ahci-test /x86_64/ahci/io/dma/lba28/short/low
PASS 6 test-crypto-tlssession /qcrypto/tlssession/altname3
PASS 7 test-crypto-tlssession /qcrypto/tlssession/altname4
PASS 8 test-crypto-tlssession /qcrypto/tlssession/altname5
==8820==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 46 ahci-test /x86_64/ahci/io/dma/lba28/short/high
PASS 9 test-crypto-tlssession /qcrypto/tlssession/altname6
==8826==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 47 ahci-test /x86_64/ahci/io/dma/lba48/simple/zero
PASS 10 test-crypto-tlssession /qcrypto/tlssession/wildcard1
==8832==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 48 ahci-test /x86_64/ahci/io/dma/lba48/simple/low
PASS 11 test-crypto-tlssession /qcrypto/tlssession/wildcard2
==8838==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 test-crypto-tlssession /qcrypto/tlssession/wildcard3
PASS 49 ahci-test /x86_64/ahci/io/dma/lba48/simple/high
PASS 13 test-crypto-tlssession /qcrypto/tlssession/wildcard4
==8844==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 test-crypto-tlssession /qcrypto/tlssession/wildcard5
PASS 15 test-crypto-tlssession /qcrypto/tlssession/wildcard6
PASS 50 ahci-test /x86_64/ahci/io/dma/lba48/double/zero
PASS 16 test-crypto-tlssession /qcrypto/tlssession/cachain
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-qga -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-qga" 
==8850==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 51 ahci-test /x86_64/ahci/io/dma/lba48/double/low
PASS 1 test-qga /qga/sync-delimited
PASS 2 test-qga /qga/sync
---
PASS 15 test-qga /qga/invalid-cmd
PASS 16 test-qga /qga/invalid-args
PASS 17 test-qga /qga/fsfreeze-status
==8862==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 52 ahci-test /x86_64/ahci/io/dma/lba48/double/high
PASS 18 test-qga /qga/blacklist
PASS 19 test-qga /qga/config
PASS 20 test-qga /qga/guest-exec
PASS 21 test-qga /qga/guest-exec-invalid
==8870==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 53 ahci-test /x86_64/ahci/io/dma/lba48/long/zero
PASS 22 test-qga /qga/guest-get-osinfo
PASS 23 test-qga /qga/guest-get-host-name
PASS 24 test-qga /qga/guest-get-timezone
PASS 25 test-qga /qga/guest-get-users
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-timed-average -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-timed-average" 
==8883==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-timed-average /timed-average/average
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-util-filemonitor -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-util-filemonitor" 
PASS 1 test-util-filemonitor /util/filemonitor
---
PASS 5 test-authz-list /auth/list/explicit/deny
PASS 6 test-authz-list /auth/list/explicit/allow
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-authz-listfile -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-authz-listfile" 
==8917==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-authz-listfile /auth/list/complex
PASS 2 test-authz-listfile /auth/list/default/deny
PASS 3 test-authz-listfile /auth/list/default/allow
---
PASS 4 test-io-channel-file /io/channel/pipe/sync
PASS 5 test-io-channel-file /io/channel/pipe/async
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-io-channel-tls -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-tls" 
==8985==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-io-channel-tls /qio/channel/tls/basic
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-io-channel-command -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-io-channel-command" 
PASS 1 test-io-channel-command /io/channel/command/fifo/sync
---
PASS 17 test-crypto-pbkdf /crypto/pbkdf/nonrfc/sha384/iter1200
PASS 18 test-crypto-pbkdf /crypto/pbkdf/nonrfc/ripemd160/iter1200
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-ivgen -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-ivgen" 
==9014==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-crypto-ivgen /crypto/ivgen/plain/1
PASS 2 test-crypto-ivgen /crypto/ivgen/plain/1f2e3d4c
PASS 3 test-crypto-ivgen /crypto/ivgen/plain/1f2e3d4c5b6a7988
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-crypto-block -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-crypto-block" 
PASS 1 test-crypto-block /crypto/block/qcow
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-logging -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-logging" 
==9040==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 test-logging /logging/parse_range
PASS 2 test-logging /logging/parse_path
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-replication -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-replication" 
==9055==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 58 ahci-test /x86_64/ahci/io/dma/lba48/short/high
PASS 1 test-replication /replication/primary/read
PASS 2 test-replication /replication/primary/write
==9060==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 59 ahci-test /x86_64/ahci/io/ncq/simple
PASS 3 test-replication /replication/primary/start
PASS 4 test-replication /replication/primary/stop
PASS 5 test-replication /replication/primary/do_checkpoint
PASS 6 test-replication /replication/primary/get_error_all
==9066==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 60 ahci-test /x86_64/ahci/io/ncq/retry
PASS 7 test-replication /replication/secondary/read
==9072==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 61 ahci-test /x86_64/ahci/flush/simple
PASS 8 test-replication /replication/secondary/write
==9078==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 62 ahci-test /x86_64/ahci/flush/retry
==9084==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9055==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffcb9f8a000; bottom 0x7fc7d24fc000; size: 0x0034e7a8e000 (227224903680)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
==9089==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 test-replication /replication/secondary/start
PASS 63 ahci-test /x86_64/ahci/flush/migrate
==9118==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9123==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 test-replication /replication/secondary/stop
PASS 64 ahci-test /x86_64/ahci/migrate/sanity
==9132==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9137==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 test-replication /replication/secondary/do_checkpoint
PASS 12 test-replication /replication/secondary/get_error_all
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  tests/test-bufferiszero -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-bufferiszero" 
PASS 65 ahci-test /x86_64/ahci/migrate/dma/simple
==9151==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9156==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 66 ahci-test /x86_64/ahci/migrate/dma/halted
==9165==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9170==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 67 ahci-test /x86_64/ahci/migrate/ncq/simple
==9179==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9184==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 68 ahci-test /x86_64/ahci/migrate/ncq/halted
==9193==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 69 ahci-test /x86_64/ahci/cdrom/eject
==9198==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 70 ahci-test /x86_64/ahci/cdrom/dma/single
==9204==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 71 ahci-test /x86_64/ahci/cdrom/dma/multi
==9210==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 72 ahci-test /x86_64/ahci/cdrom/pio/single
==9216==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
==9216==WARNING: ASan is ignoring requested __asan_handle_no_return: stack top: 0x7ffe13956000; bottom 0x7f6c113fe000; size: 0x009202558000 (627104382976)
False positive error reports may follow
For details see https://github.com/google/sanitizers/issues/189
PASS 73 ahci-test /x86_64/ahci/cdrom/pio/multi
==9222==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 74 ahci-test /x86_64/ahci/cdrom/pio/bcl
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/hd-geo-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="hd-geo-test" 
PASS 1 hd-geo-test /x86_64/hd-geo/ide/none
==9236==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 hd-geo-test /x86_64/hd-geo/ide/drive/cd_0
==9242==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/blank
==9248==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/lba
==9254==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 hd-geo-test /x86_64/hd-geo/ide/drive/mbr/chs
==9260==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 hd-geo-test /x86_64/hd-geo/ide/device/mbr/blank
==9266==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 hd-geo-test /x86_64/hd-geo/ide/device/mbr/lba
==9272==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 hd-geo-test /x86_64/hd-geo/ide/device/mbr/chs
==9278==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 hd-geo-test /x86_64/hd-geo/ide/device/user/chs
==9283==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 hd-geo-test /x86_64/hd-geo/ide/device/user/chst
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-order-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-order-test" 
PASS 1 test-bufferiszero /cutils/bufferiszero
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9368==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 bios-tables-test /x86_64/acpi/piix4
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9374==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 bios-tables-test /x86_64/acpi/q35
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9380==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 bios-tables-test /x86_64/acpi/piix4/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9386==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 bios-tables-test /x86_64/acpi/piix4/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9392==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 5 bios-tables-test /x86_64/acpi/piix4/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9399==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 bios-tables-test /x86_64/acpi/piix4/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9405==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 bios-tables-test /x86_64/acpi/piix4/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9411==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 bios-tables-test /x86_64/acpi/piix4/dimmpxm
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9420==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 9 bios-tables-test /x86_64/acpi/q35/bridge
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9426==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 10 bios-tables-test /x86_64/acpi/q35/mmio64
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9432==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 11 bios-tables-test /x86_64/acpi/q35/ipmi
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9438==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 12 bios-tables-test /x86_64/acpi/q35/cpuhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9445==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 13 bios-tables-test /x86_64/acpi/q35/memhp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9451==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 14 bios-tables-test /x86_64/acpi/q35/numamem
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9457==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 15 bios-tables-test /x86_64/acpi/q35/dimmpxm
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/boot-serial-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="boot-serial-test" 
PASS 1 boot-serial-test /x86_64/boot-serial/isapc
---
PASS 1 i440fx-test /x86_64/i440fx/defaults
PASS 2 i440fx-test /x86_64/i440fx/pam
PASS 3 i440fx-test /x86_64/i440fx/firmware/bios
==9541==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 i440fx-test /x86_64/i440fx/firmware/pflash
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/fw_cfg-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="fw_cfg-test" 
PASS 1 fw_cfg-test /x86_64/fw_cfg/signature
---
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/drive_del-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="drive_del-test" 
PASS 1 drive_del-test /x86_64/drive_del/without-dev
PASS 2 drive_del-test /x86_64/drive_del/after_failed_device_add
==9629==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 drive_del-test /x86_64/blockdev/drive_del_device_del
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/wdt_ib700-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="wdt_ib700-test" 
PASS 1 wdt_ib700-test /x86_64/wdt_ib700/pause
---
PASS 1 usb-hcd-uhci-test /x86_64/uhci/pci/init
PASS 2 usb-hcd-uhci-test /x86_64/uhci/pci/port1
PASS 3 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug
==9824==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 usb-hcd-uhci-test /x86_64/uhci/pci/hotplug/usb-storage
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/usb-hcd-xhci-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="usb-hcd-xhci-test" 
PASS 1 usb-hcd-xhci-test /x86_64/xhci/pci/init
PASS 2 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug
==9833==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-uas
PASS 4 usb-hcd-xhci-test /x86_64/xhci/pci/hotplug/usb-ccid
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/cpu-plug-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="cpu-plug-test" 
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9939==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 1 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9945==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 vmgenid-test /x86_64/vmgenid/vmgenid/set-guid-auto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==9951==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 vmgenid-test /x86_64/vmgenid/vmgenid/query-monitor
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/tpm-crb-swtpm-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="tpm-crb-swtpm-test" 
SKIP 1 tpm-crb-swtpm-test /x86_64/tpm/crb-swtpm/test # SKIP swtpm not in PATH or missing --tpm2 support
---
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10056==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10061==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 3 migration-test /x86_64/migration/fd_proto
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10069==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10074==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 4 migration-test /x86_64/migration/postcopy/unix
PASS 5 migration-test /x86_64/migration/postcopy/recovery
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10104==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10109==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 6 migration-test /x86_64/migration/precopy/unix
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10118==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10123==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 7 migration-test /x86_64/migration/precopy/tcp
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10132==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
Could not access KVM kernel module: No such file or directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
==10137==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 8 migration-test /x86_64/migration/xbzrle/unix
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/test-x86-cpuid-compat -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="test-x86-cpuid-compat" 
PASS 1 test-x86-cpuid-compat /x86/cpuid/parsing-plus-minus
---
PASS 6 numa-test /x86_64/numa/pc/dynamic/cpu
MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/qmp-test -m=quick -k --tap < /dev/null | ./scripts/tap-driver.pl --test-name="qmp-test" 
PASS 1 qmp-test /x86_64/qmp/protocol
==10466==WARNING: ASan doesn't fully support makecontext/swapcontext functions and may produce false positives in some cases!
PASS 2 qmp-test /x86_64/qmp/oob
PASS 3 qmp-test /x86_64/qmp/preconfig
PASS 4 qmp-test /x86_64/qmp/missing-any-arg
---
PASS 5 device-introspect-test /x86_64/device/introspect/abstract-interfaces

=================================================================
==10714==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x56353884cb7e in calloc (/tmp/qemu-test/build/x86_64-softmmu/qemu-system-x86_64+0x19fcb7e)
---

SUMMARY: AddressSanitizer: 64 byte(s) leaked in 2 allocation(s).
/tmp/qemu-test/src/tests/libqtest.c:137: kill_qemu() tried to terminate QEMU process but encountered exit status 1
ERROR - too few tests run (expected 6, got 5)
make: *** [/tmp/qemu-test/src/tests/Makefile.include:896: check-qtest-x86_64] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):


The full log is available at
http://patchew.org/logs/cover.1562154272.git.qemu_oss@crudebyte.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes'
  2019-07-03 11:13 ` [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes' Christian Schoenebeck via Qemu-devel
@ 2019-07-06 10:22   ` Christian Schoenebeck via Qemu-devel
  2019-08-30 11:48     ` Greg Kurz
  0 siblings, 1 reply; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-07-06 10:22 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P. Berrangé,
	Christian Schoenebeck, Greg Kurz, Antonios Motakis

On Mittwoch, 3. Juli 2019 13:13:26 CEST Christian Schoenebeck wrote:
> To support multiple devices on the 9p share, and avoid
> qid path collisions we take the device id as input
[snip]
>      - Fixed v9fs_do_readdir() having exposed info outside
>        export root with '..' entry.
[snip]
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 8cc65c2c67..39c6c2a894 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
[snip]
> @@ -1940,6 +2041,19 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu,
> V9fsFidState *fidp, int32_t count = 0;
>      off_t saved_dir_pos;
>      struct dirent *dent;
> +    struct stat stbuf;
> +    bool fidIsExportRoot;
> +
> +    /*
> +     * determine if fidp is the export root, which is required for safe
> +     * handling of ".." below
> +     */
> +    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
> +    if (err < 0) {
> +        return err;
> +    }
> +    fidIsExportRoot = pdu->s->dev_id == stbuf.st_dev &&
> +                      pdu->s->root_ino == stbuf.st_ino;
> 
>      /* save the directory position */
>      saved_dir_pos = v9fs_co_telldir(pdu, fidp);
> @@ -1964,16 +2078,51 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU
> *pdu, V9fsFidState *fidp, v9fs_string_free(&name);
>              return count;
>          }
> -        /*
> -         * Fill up just the path field of qid because the client uses
> -         * only that. To fill the entire qid structure we will have
> -         * to stat each dirent found, which is expensive
> -         */
> -        size = MIN(sizeof(dent->d_ino), sizeof(qid.path));
> -        memcpy(&qid.path, &dent->d_ino, size);
> -        /* Fill the other fields with dummy values */
> -        qid.type = 0;
> -        qid.version = 0;
> +
> +        if (fidIsExportRoot && !strcmp("..", dent->d_name)) {
> +            /*
> +             * if "." is export root, then return qid of export root for
> +             * ".." to avoid exposing anything outside the export
> +             */
> +            err = fid_to_qid(pdu, fidp, &qid);
> +            if (err < 0) {
> +                v9fs_readdir_unlock(&fidp->fs.dir);
> +                v9fs_co_seekdir(pdu, fidp, saved_dir_pos);
> +                v9fs_string_free(&name);
> +                return err;
> +            }

Hmm, I start to wonder whether I should postpone that particular bug fix and 
not make it part of that QID fix patch series (not even as separate patch 
there). Because that fix needs some more adjustments. E.g. I should adjust 
dent->d_type here as well; but more notably it should also distinguish between 
the case where the export root is mounted as / on guest or not and that's 
where this fix could become ugly and grow in size.

To make the case clear:  calling on guest	

	readdir(pathOfSome9pExportRootOnGuest);

currently always returns for its ".." result entry the inode number and d_type 
of the export root's parent directory on host, so it exposes information of 
host outside the 9p export.

I don't see that as security issue, since the information revealed is limited 
to the inode number and d_type, but it is definitely incorrect behaviour.

Best regards,
Christian Schoenebeck


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path
  2019-07-03 10:55 ` [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path Christian Schoenebeck via Qemu-devel
@ 2019-07-31 19:14   ` Greg Kurz
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kurz @ 2019-07-31 19:14 UTC (permalink / raw)
  To: Christian Schoenebeck via Qemu-devel
  Cc: Daniel P. Berrangé, Christian Schoenebeck, Antonios Motakis

On Wed, 3 Jul 2019 12:55:45 +0200
Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org> wrote:

> There is no need for signedness on these QID fields for 9p.
> 
> Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
> [CS: - Also make QID type unsigned.
>      - Adjust donttouch_stat() to new types.
>      - Adjust trace-events to new types. ]
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---

Applied to https://github.com/gkurz/qemu/commits/9p-next .

Thanks,

--
Greg

>  fsdev/9p-marshal.h   |  6 +++---
>  hw/9pfs/9p.c         |  6 +++---
>  hw/9pfs/trace-events | 14 +++++++-------
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
> index c8823d878f..8f3babb60a 100644
> --- a/fsdev/9p-marshal.h
> +++ b/fsdev/9p-marshal.h
> @@ -9,9 +9,9 @@ typedef struct V9fsString
>  
>  typedef struct V9fsQID
>  {
> -    int8_t type;
> -    int32_t version;
> -    int64_t path;
> +    uint8_t type;
> +    uint32_t version;
> +    uint64_t path;
>  } V9fsQID;
>  
>  typedef struct V9fsStat
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 55821343e5..586a6dccba 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -743,9 +743,9 @@ static int donttouch_stat(V9fsStat *stat)
>  {
>      if (stat->type == -1 &&
>          stat->dev == -1 &&
> -        stat->qid.type == -1 &&
> -        stat->qid.version == -1 &&
> -        stat->qid.path == -1 &&
> +        stat->qid.type == 0xff &&
> +        stat->qid.version == (uint32_t) -1 &&
> +        stat->qid.path == (uint64_t) -1 &&
>          stat->mode == -1 &&
>          stat->atime == -1 &&
>          stat->mtime == -1 &&
> diff --git a/hw/9pfs/trace-events b/hw/9pfs/trace-events
> index c0a0a4ab5d..10188daf7f 100644
> --- a/hw/9pfs/trace-events
> +++ b/hw/9pfs/trace-events
> @@ -6,7 +6,7 @@ v9fs_rerror(uint16_t tag, uint8_t id, int err) "tag %d id %d err %d"
>  v9fs_version(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
>  v9fs_version_return(uint16_t tag, uint8_t id, int32_t msize, char* version) "tag %d id %d msize %d version %s"
>  v9fs_attach(uint16_t tag, uint8_t id, int32_t fid, int32_t afid, char* uname, char* aname) "tag %u id %u fid %d afid %d uname %s aname %s"
> -v9fs_attach_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path) "tag %d id %d type %d version %d path %"PRId64
> +v9fs_attach_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path) "tag %u id %u type %u version %u path %"PRIu64
>  v9fs_stat(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
>  v9fs_stat_return(uint16_t tag, uint8_t id, int32_t mode, int32_t atime, int32_t mtime, int64_t length) "tag %d id %d stat={mode %d atime %d mtime %d length %"PRId64"}"
>  v9fs_getattr(uint16_t tag, uint8_t id, int32_t fid, uint64_t request_mask) "tag %d id %d fid %d request_mask %"PRIu64
> @@ -14,9 +14,9 @@ v9fs_getattr_return(uint16_t tag, uint8_t id, uint64_t result_mask, uint32_t mod
>  v9fs_walk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, uint16_t nwnames) "tag %d id %d fid %d newfid %d nwnames %d"
>  v9fs_walk_return(uint16_t tag, uint8_t id, uint16_t nwnames, void* qids) "tag %d id %d nwnames %d qids %p"
>  v9fs_open(uint16_t tag, uint8_t id, int32_t fid, int32_t mode) "tag %d id %d fid %d mode %d"
> -v9fs_open_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} iounit %d"
> +v9fs_open_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d"
>  v9fs_lcreate(uint16_t tag, uint8_t id, int32_t dfid, int32_t flags, int32_t mode, uint32_t gid) "tag %d id %d dfid %d flags %d mode %d gid %u"
> -v9fs_lcreate_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int32_t iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} iounit %d"
> +v9fs_lcreate_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int32_t iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d"
>  v9fs_fsync(uint16_t tag, uint8_t id, int32_t fid, int datasync) "tag %d id %d fid %d datasync %d"
>  v9fs_clunk(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
>  v9fs_read(uint16_t tag, uint8_t id, int32_t fid, uint64_t off, uint32_t max_count) "tag %d id %d fid %d off %"PRIu64" max_count %u"
> @@ -26,21 +26,21 @@ v9fs_readdir_return(uint16_t tag, uint8_t id, uint32_t count, ssize_t retval) "t
>  v9fs_write(uint16_t tag, uint8_t id, int32_t fid, uint64_t off, uint32_t count, int cnt) "tag %d id %d fid %d off %"PRIu64" count %u cnt %d"
>  v9fs_write_return(uint16_t tag, uint8_t id, int32_t total, ssize_t err) "tag %d id %d total %d err %zd"
>  v9fs_create(uint16_t tag, uint8_t id, int32_t fid, char* name, int32_t perm, int8_t mode) "tag %d id %d fid %d name %s perm %d mode %d"
> -v9fs_create_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int iounit) "tag %d id %d qid={type %d version %d path %"PRId64"} iounit %d"
> +v9fs_create_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int iounit) "tag %u id %u qid={type %u version %u path %"PRIu64"} iounit %d"
>  v9fs_symlink(uint16_t tag, uint8_t id, int32_t fid,  char* name, char* symname, uint32_t gid) "tag %d id %d fid %d name %s symname %s gid %u"
> -v9fs_symlink_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path) "tag %d id %d qid={type %d version %d path %"PRId64"}"
> +v9fs_symlink_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path) "tag %u id %u qid={type %u version %u path %"PRIu64"}"
>  v9fs_flush(uint16_t tag, uint8_t id, int16_t flush_tag) "tag %d id %d flush_tag %d"
>  v9fs_link(uint16_t tag, uint8_t id, int32_t dfid, int32_t oldfid, char* name) "tag %d id %d dfid %d oldfid %d name %s"
>  v9fs_remove(uint16_t tag, uint8_t id, int32_t fid) "tag %d id %d fid %d"
>  v9fs_wstat(uint16_t tag, uint8_t id, int32_t fid, int32_t mode, int32_t atime, int32_t mtime) "tag %u id %u fid %d stat={mode %d atime %d mtime %d}"
>  v9fs_mknod(uint16_t tag, uint8_t id, int32_t fid, int mode, int major, int minor) "tag %d id %d fid %d mode %d major %d minor %d"
> -v9fs_mknod_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path) "tag %d id %d qid={type %d version %d path %"PRId64"}"
> +v9fs_mknod_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path) "tag %u id %u qid={type %u version %u path %"PRIu64"}"
>  v9fs_lock(uint16_t tag, uint8_t id, int32_t fid, uint8_t type, uint64_t start, uint64_t length) "tag %d id %d fid %d type %d start %"PRIu64" length %"PRIu64
>  v9fs_lock_return(uint16_t tag, uint8_t id, int8_t status) "tag %d id %d status %d"
>  v9fs_getlock(uint16_t tag, uint8_t id, int32_t fid, uint8_t type, uint64_t start, uint64_t length)"tag %d id %d fid %d type %d start %"PRIu64" length %"PRIu64
>  v9fs_getlock_return(uint16_t tag, uint8_t id, uint8_t type, uint64_t start, uint64_t length, uint32_t proc_id) "tag %d id %d type %d start %"PRIu64" length %"PRIu64" proc_id %u"
>  v9fs_mkdir(uint16_t tag, uint8_t id, int32_t fid, char* name, int mode, uint32_t gid) "tag %u id %u fid %d name %s mode %d gid %u"
> -v9fs_mkdir_return(uint16_t tag, uint8_t id, int8_t type, int32_t version, int64_t path, int err) "tag %u id %u qid={type %d version %d path %"PRId64"} err %d"
> +v9fs_mkdir_return(uint16_t tag, uint8_t id, uint8_t type, uint32_t version, uint64_t path, int err) "tag %u id %u qid={type %u version %u path %"PRIu64"} err %d"
>  v9fs_xattrwalk(uint16_t tag, uint8_t id, int32_t fid, int32_t newfid, char* name) "tag %d id %d fid %d newfid %d name %s"
>  v9fs_xattrwalk_return(uint16_t tag, uint8_t id, int64_t size) "tag %d id %d size %"PRId64
>  v9fs_xattrcreate(uint16_t tag, uint8_t id, int32_t fid, char* name, uint64_t size, int flags) "tag %d id %d fid %d name %s size %"PRIu64" flags %d"



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/5] 9p: Treat multiple devices on one export as an error
  2019-07-03 11:01 ` [Qemu-devel] [PATCH v5 2/5] 9p: Treat multiple devices on one export as an error Christian Schoenebeck via Qemu-devel
@ 2019-07-31 20:30   ` Greg Kurz
  0 siblings, 0 replies; 13+ messages in thread
From: Greg Kurz @ 2019-07-31 20:30 UTC (permalink / raw)
  To: Christian Schoenebeck via Qemu-devel
  Cc: Daniel P. Berrangé,
	Christian Schoenebeck, Dr. David Alan Gilbert, Antonios Motakis,
	Stefan Hajnoczi

On Wed, 3 Jul 2019 13:01:34 +0200
Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org> wrote:

> The QID path should uniquely identify a file. However, the
> inode of a file is currently used as the QID path, which
> on its own only uniquely identifies files within a device.
> Here we track the device hosting the 9pfs share, in order
> to prevent security issues with QID path collisions from
> other devices.
> 
> Signed-off-by: Antonios Motakis <antonios.motakis@huawei.com>
> [CS: - Assign dev_id to export root's device already in
>        v9fs_device_realize_common(), not postponed in
>        stat_to_qid().
>      - error_report_once() if more than one device was
>        shared by export.
>      - Return -ENODEV instead of -ENOSYS in stat_to_qid().
>      - Fixed typo in log comment. ]
> Signed-off-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> ---

The fix is good in the sense that it purely forbids cross-device accesses,
which provides improved security. But some people may wish to do cross-device
anyway, for example because they have ultimate knowledge or luck that their
usual setup doesn't have colliding inodes, or simply that their workload
doesn't care for inode numbers at all.

Next patches in this series introduce inode number remapping, which is
a good way of addressing collisions. This should always be used in a
cross-device setup. But again, people who don't do cross-device, may
wish to disable remapping to avoid the potential performance loss or
extra memory consumption.

I suggest you add a new property to the 9p device so that people can
choose the behavior that best fits their need. Something like:

-device virtio-9p,cross-device=[ignore|forbid|remap]

ignore: detect the cross-device condition and spit-once a big fat warning
        about the potential consequences, and alternatives to avoid
        them, but don't fail the I/O
forbid: detect the cross-device condition, spit-once an error and fail the
        I/O like the current patch does
remap: well... remap all inode numbers

I would make 'ignore' the default since it is the current behavior, but
I'm ready to change my mind if someone has a convincing argument that
the default should be more secure.

Cc'ing Stefan and Dave in case they have some more suggestions after the
discussion we had on irc.

>  hw/9pfs/9p.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++------------
>  hw/9pfs/9p.h |  1 +
>  2 files changed, 56 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> index 586a6dccba..8cc65c2c67 100644
> --- a/hw/9pfs/9p.c
> +++ b/hw/9pfs/9p.c
> @@ -572,10 +572,18 @@ static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
>                                  P9_STAT_MODE_SOCKET)
>  
>  /* This is the algorithm from ufs in spfs */
> -static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
> +static int stat_to_qid(V9fsPDU *pdu, const struct stat *stbuf, V9fsQID *qidp)
>  {
>      size_t size;
>  
> +    if (pdu->s->dev_id != stbuf->st_dev) {
> +        error_report_once(
> +            "9p: Multiple devices detected in same VirtFS export. "
> +            "You must use a separate export for each device."
> +        );
> +        return -ENODEV;
> +    }
> +
>      memset(&qidp->path, 0, sizeof(qidp->path));
>      size = MIN(sizeof(stbuf->st_ino), sizeof(qidp->path));
>      memcpy(&qidp->path, &stbuf->st_ino, size);
> @@ -587,6 +595,8 @@ static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
>      if (S_ISLNK(stbuf->st_mode)) {
>          qidp->type |= P9_QID_TYPE_SYMLINK;
>      }
> +
> +    return 0;
>  }
>  
>  static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
> @@ -599,7 +609,10 @@ static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
>      if (err < 0) {
>          return err;
>      }
> -    stat_to_qid(&stbuf, qidp);
> +    err = stat_to_qid(pdu, &stbuf, qidp);
> +    if (err < 0) {
> +        return err;
> +    }
>      return 0;
>  }
>  
> @@ -830,7 +843,10 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,
>  
>      memset(v9stat, 0, sizeof(*v9stat));
>  
> -    stat_to_qid(stbuf, &v9stat->qid);
> +    err = stat_to_qid(pdu, stbuf, &v9stat->qid);
> +    if (err < 0) {
> +        return err;
> +    }
>      v9stat->mode = stat_to_v9mode(stbuf);
>      v9stat->atime = stbuf->st_atime;
>      v9stat->mtime = stbuf->st_mtime;
> @@ -891,7 +907,7 @@ static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *path,
>  #define P9_STATS_ALL           0x00003fffULL /* Mask for All fields above */
>  
>  
> -static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
> +static int stat_to_v9stat_dotl(V9fsPDU *pdu, const struct stat *stbuf,
>                                  V9fsStatDotl *v9lstat)
>  {
>      memset(v9lstat, 0, sizeof(*v9lstat));
> @@ -913,7 +929,7 @@ static void stat_to_v9stat_dotl(V9fsState *s, const struct stat *stbuf,
>      /* Currently we only support BASIC fields in stat */
>      v9lstat->st_result_mask = P9_STATS_BASIC;
>  
> -    stat_to_qid(stbuf, &v9lstat->qid);
> +    return stat_to_qid(pdu, stbuf, &v9lstat->qid);
>  }
>  
>  static void print_sg(struct iovec *sg, int cnt)
> @@ -1115,7 +1131,6 @@ static void coroutine_fn v9fs_getattr(void *opaque)
>      uint64_t request_mask;
>      V9fsStatDotl v9stat_dotl;
>      V9fsPDU *pdu = opaque;
> -    V9fsState *s = pdu->s;
>  
>      retval = pdu_unmarshal(pdu, offset, "dq", &fid, &request_mask);
>      if (retval < 0) {
> @@ -1136,7 +1151,10 @@ static void coroutine_fn v9fs_getattr(void *opaque)
>      if (retval < 0) {
>          goto out;
>      }
> -    stat_to_v9stat_dotl(s, &stbuf, &v9stat_dotl);
> +    retval = stat_to_v9stat_dotl(pdu, &stbuf, &v9stat_dotl);
> +    if (retval < 0) {
> +        goto out;
> +    }
>  
>      /*  fill st_gen if requested and supported by underlying fs */
>      if (request_mask & P9_STATS_GEN) {
> @@ -1381,7 +1399,10 @@ static void coroutine_fn v9fs_walk(void *opaque)
>              if (err < 0) {
>                  goto out;
>              }
> -            stat_to_qid(&stbuf, &qid);
> +            err = stat_to_qid(pdu, &stbuf, &qid);
> +            if (err < 0) {
> +                goto out;
> +            }
>              v9fs_path_copy(&dpath, &path);
>          }
>          memcpy(&qids[name_idx], &qid, sizeof(qid));
> @@ -1483,7 +1504,10 @@ static void coroutine_fn v9fs_open(void *opaque)
>      if (err < 0) {
>          goto out;
>      }
> -    stat_to_qid(&stbuf, &qid);
> +    err = stat_to_qid(pdu, &stbuf, &qid);
> +    if (err < 0) {
> +        goto out;
> +    }
>      if (S_ISDIR(stbuf.st_mode)) {
>          err = v9fs_co_opendir(pdu, fidp);
>          if (err < 0) {
> @@ -1593,7 +1617,10 @@ static void coroutine_fn v9fs_lcreate(void *opaque)
>          fidp->flags |= FID_NON_RECLAIMABLE;
>      }
>      iounit =  get_iounit(pdu, &fidp->path);
> -    stat_to_qid(&stbuf, &qid);
> +    err = stat_to_qid(pdu, &stbuf, &qid);
> +    if (err < 0) {
> +        goto out;
> +    }
>      err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
>      if (err < 0) {
>          goto out;
> @@ -2327,7 +2354,10 @@ static void coroutine_fn v9fs_create(void *opaque)
>          }
>      }
>      iounit = get_iounit(pdu, &fidp->path);
> -    stat_to_qid(&stbuf, &qid);
> +    err = stat_to_qid(pdu, &stbuf, &qid);
> +    if (err < 0) {
> +        goto out;
> +    }
>      err = pdu_marshal(pdu, offset, "Qd", &qid, iounit);
>      if (err < 0) {
>          goto out;
> @@ -2384,7 +2414,10 @@ static void coroutine_fn v9fs_symlink(void *opaque)
>      if (err < 0) {
>          goto out;
>      }
> -    stat_to_qid(&stbuf, &qid);
> +    err = stat_to_qid(pdu, &stbuf, &qid);
> +    if (err < 0) {
> +        goto out;
> +    }
>      err =  pdu_marshal(pdu, offset, "Q", &qid);
>      if (err < 0) {
>          goto out;
> @@ -3064,7 +3097,10 @@ static void coroutine_fn v9fs_mknod(void *opaque)
>      if (err < 0) {
>          goto out;
>      }
> -    stat_to_qid(&stbuf, &qid);
> +    err = stat_to_qid(pdu, &stbuf, &qid);
> +    if (err < 0) {
> +        goto out;
> +    }
>      err = pdu_marshal(pdu, offset, "Q", &qid);
>      if (err < 0) {
>          goto out;
> @@ -3222,7 +3258,10 @@ static void coroutine_fn v9fs_mkdir(void *opaque)
>      if (err < 0) {
>          goto out;
>      }
> -    stat_to_qid(&stbuf, &qid);
> +    err = stat_to_qid(pdu, &stbuf, &qid);
> +    if (err < 0) {
> +        goto out;
> +    }
>      err = pdu_marshal(pdu, offset, "Q", &qid);
>      if (err < 0) {
>          goto out;
> @@ -3633,6 +3672,8 @@ int v9fs_device_realize_common(V9fsState *s, const V9fsTransport *t,
>          goto out;
>      }
>  
> +    s->dev_id = stat.st_dev;
> +
>      s->ctx.fst = &fse->fst;
>      fsdev_throttle_init(s->ctx.fst);
>  
> diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
> index 8883761b2c..5e316178d5 100644
> --- a/hw/9pfs/9p.h
> +++ b/hw/9pfs/9p.h
> @@ -256,6 +256,7 @@ struct V9fsState
>      Error *migration_blocker;
>      V9fsConf fsconf;
>      V9fsQID root_qid;
> +    dev_t dev_id;
>  };
>  
>  /* 9p2000.L open flags */



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes'
  2019-07-06 10:22   ` Christian Schoenebeck via Qemu-devel
@ 2019-08-30 11:48     ` Greg Kurz
  2019-09-01 19:35       ` Christian Schoenebeck via Qemu-devel
  0 siblings, 1 reply; 13+ messages in thread
From: Greg Kurz @ 2019-08-30 11:48 UTC (permalink / raw)
  To: qemu-devel
  Cc: Daniel P. Berrangé, Christian Schoenebeck, Antonios Motakis

On Sat, 06 Jul 2019 12:22:27 +0200
Christian Schoenebeck via Qemu-devel <qemu-devel@nongnu.org> wrote:

> On Mittwoch, 3. Juli 2019 13:13:26 CEST Christian Schoenebeck wrote:
> > To support multiple devices on the 9p share, and avoid
> > qid path collisions we take the device id as input
> [snip]
> >      - Fixed v9fs_do_readdir() having exposed info outside
> >        export root with '..' entry.
> [snip]
> > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > index 8cc65c2c67..39c6c2a894 100644
> > --- a/hw/9pfs/9p.c
> > +++ b/hw/9pfs/9p.c
> [snip]
> > @@ -1940,6 +2041,19 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu,
> > V9fsFidState *fidp, int32_t count = 0;
> >      off_t saved_dir_pos;
> >      struct dirent *dent;
> > +    struct stat stbuf;
> > +    bool fidIsExportRoot;
> > +
> > +    /*
> > +     * determine if fidp is the export root, which is required for safe
> > +     * handling of ".." below
> > +     */
> > +    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
> > +    if (err < 0) {
> > +        return err;
> > +    }
> > +    fidIsExportRoot = pdu->s->dev_id == stbuf.st_dev &&
> > +                      pdu->s->root_ino == stbuf.st_ino;
> > 
> >      /* save the directory position */
> >      saved_dir_pos = v9fs_co_telldir(pdu, fidp);
> > @@ -1964,16 +2078,51 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU
> > *pdu, V9fsFidState *fidp, v9fs_string_free(&name);
> >              return count;
> >          }
> > -        /*
> > -         * Fill up just the path field of qid because the client uses
> > -         * only that. To fill the entire qid structure we will have
> > -         * to stat each dirent found, which is expensive
> > -         */
> > -        size = MIN(sizeof(dent->d_ino), sizeof(qid.path));
> > -        memcpy(&qid.path, &dent->d_ino, size);
> > -        /* Fill the other fields with dummy values */
> > -        qid.type = 0;
> > -        qid.version = 0;
> > +
> > +        if (fidIsExportRoot && !strcmp("..", dent->d_name)) {
> > +            /*
> > +             * if "." is export root, then return qid of export root for
> > +             * ".." to avoid exposing anything outside the export
> > +             */
> > +            err = fid_to_qid(pdu, fidp, &qid);
> > +            if (err < 0) {
> > +                v9fs_readdir_unlock(&fidp->fs.dir);
> > +                v9fs_co_seekdir(pdu, fidp, saved_dir_pos);
> > +                v9fs_string_free(&name);
> > +                return err;
> > +            }
> 
> Hmm, I start to wonder whether I should postpone that particular bug fix and 
> not make it part of that QID fix patch series (not even as separate patch 
> there). Because that fix needs some more adjustments. E.g. I should adjust 
> dent->d_type here as well; but more notably it should also distinguish between 
> the case where the export root is mounted as / on guest or not and that's 
> where this fix could become ugly and grow in size.
> 
> To make the case clear:  calling on guest	
> 
> 	readdir(pathOfSome9pExportRootOnGuest);
> 
> currently always returns for its ".." result entry the inode number and d_type 
> of the export root's parent directory on host, so it exposes information of 
> host outside the 9p export.
> 
> I don't see that as security issue, since the information revealed is limited 
> to the inode number and d_type, but it is definitely incorrect behaviour.
> 

Definitely. This should be fixed independently of this series. Maybe follow
the same approach as commit 56f101ecce0e "9pfs: handle walk of ".." in the
root directory", ie. basically make /.. an alias of /.

> Best regards,
> Christian Schoenebeck
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes'
  2019-08-30 11:48     ` Greg Kurz
@ 2019-09-01 19:35       ` Christian Schoenebeck via Qemu-devel
  0 siblings, 0 replies; 13+ messages in thread
From: Christian Schoenebeck via Qemu-devel @ 2019-09-01 19:35 UTC (permalink / raw)
  To: qemu-devel
  Cc: Christian Schoenebeck, Daniel P. Berrangé,
	Greg Kurz, Antonios Motakis

On Freitag, 30. August 2019 13:48:27 CEST Greg Kurz wrote:
> > > diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
> > > index 8cc65c2c67..39c6c2a894 100644
> > > --- a/hw/9pfs/9p.c
> > > +++ b/hw/9pfs/9p.c
> > 
> > [snip]
> > 
> > > @@ -1940,6 +2041,19 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU
> > > *pdu, V9fsFidState *fidp, int32_t count = 0;
> > > 
> > >      off_t saved_dir_pos;
> > >      struct dirent *dent;
> > > 
> > > +    struct stat stbuf;
> > > +    bool fidIsExportRoot;
> > > +
> > > +    /*
> > > +     * determine if fidp is the export root, which is required for safe
> > > +     * handling of ".." below
> > > +     */
> > > +    err = v9fs_co_lstat(pdu, &fidp->path, &stbuf);
> > > +    if (err < 0) {
> > > +        return err;
> > > +    }
> > > +    fidIsExportRoot = pdu->s->dev_id == stbuf.st_dev &&
> > > +                      pdu->s->root_ino == stbuf.st_ino;
> > > 
> > >      /* save the directory position */
> > >      saved_dir_pos = v9fs_co_telldir(pdu, fidp);
> > > 
> > > @@ -1964,16 +2078,51 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU
> > > *pdu, V9fsFidState *fidp, v9fs_string_free(&name);
> > > 
> > >              return count;
> > >          
> > >          }
> > > 
> > > -        /*
> > > -         * Fill up just the path field of qid because the client uses
> > > -         * only that. To fill the entire qid structure we will have
> > > -         * to stat each dirent found, which is expensive
> > > -         */
> > > -        size = MIN(sizeof(dent->d_ino), sizeof(qid.path));
> > > -        memcpy(&qid.path, &dent->d_ino, size);
> > > -        /* Fill the other fields with dummy values */
> > > -        qid.type = 0;
> > > -        qid.version = 0;
> > > +
> > > +        if (fidIsExportRoot && !strcmp("..", dent->d_name)) {
> > > +            /*
> > > +             * if "." is export root, then return qid of export root
> > > for
> > > +             * ".." to avoid exposing anything outside the export
> > > +             */
> > > +            err = fid_to_qid(pdu, fidp, &qid);
> > > +            if (err < 0) {
> > > +                v9fs_readdir_unlock(&fidp->fs.dir);
> > > +                v9fs_co_seekdir(pdu, fidp, saved_dir_pos);
> > > +                v9fs_string_free(&name);
> > > +                return err;
> > > +            }
> > 
> > Hmm, I start to wonder whether I should postpone that particular bug fix
> > and not make it part of that QID fix patch series (not even as separate
> > patch there). Because that fix needs some more adjustments. E.g. I should
> > adjust dent->d_type here as well; but more notably it should also
> > distinguish between the case where the export root is mounted as / on
> > guest or not and that's where this fix could become ugly and grow in
> > size.
> > 
> > To make the case clear:  calling on guest
> > 
> > 	readdir(pathOfSome9pExportRootOnGuest);
> > 
> > currently always returns for its ".." result entry the inode number and
> > d_type of the export root's parent directory on host, so it exposes
> > information of host outside the 9p export.
> > 
> > I don't see that as security issue, since the information revealed is
> > limited to the inode number and d_type, but it is definitely incorrect
> > behaviour.
> Definitely. This should be fixed independently of this series. Maybe follow
> the same approach as commit 56f101ecce0e "9pfs: handle walk of ".." in the
> root directory", ie. basically make /.. an alias of /.

That's actually what the suggested fix already did in v5 here (see diff above). 
However I was worried whether I thought about all edge cases. So I also need 
some more testing and hence clearly decided to postpone this fix for now.

Best regards,
Christian Schoenebeck



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2019-09-01 19:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-03 11:44 [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions Christian Schoenebeck via Qemu-devel
2019-07-03 10:55 ` [Qemu-devel] [PATCH v5 1/5] 9p: unsigned type for type, version, path Christian Schoenebeck via Qemu-devel
2019-07-31 19:14   ` Greg Kurz
2019-07-03 11:01 ` [Qemu-devel] [PATCH v5 2/5] 9p: Treat multiple devices on one export as an error Christian Schoenebeck via Qemu-devel
2019-07-31 20:30   ` Greg Kurz
2019-07-03 11:13 ` [Qemu-devel] [PATCH v5 3/5] 9p: Added virtfs option 'remap_inodes' Christian Schoenebeck via Qemu-devel
2019-07-06 10:22   ` Christian Schoenebeck via Qemu-devel
2019-08-30 11:48     ` Greg Kurz
2019-09-01 19:35       ` Christian Schoenebeck via Qemu-devel
2019-07-03 11:17 ` [Qemu-devel] [PATCH v5 4/5] 9p: stat_to_qid: implement slow path Christian Schoenebeck via Qemu-devel
2019-07-03 11:27 ` [Qemu-devel] [PATCH v5 5/5] 9p: Use variable length suffixes for inode remapping Christian Schoenebeck via Qemu-devel
2019-07-03 15:46 ` [Qemu-devel] [PATCH v5 0/5] 9p: Fix file ID collisions no-reply
2019-07-03 16:04 ` no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.