[PATCH 1/4] kvm tools: fix sigsegv in irq__exit

[PATCH 1/4] kvm tools: fix sigsegv in irq__exit

[Sasha Levin]

From: Sasha Levin <levinsasha928@gmail.com>

We free the structure, but never remove them from the tree or list, then
we freed them the next time we ran through that structure.

This patch also simplifies irq__exit a bit.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/x86/irq.c |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/tools/kvm/x86/irq.c b/tools/kvm/x86/irq.c
index dc07f28..e83df99 100644
--- a/tools/kvm/x86/irq.c
+++ b/tools/kvm/x86/irq.c
@@ -179,25 +179,18 @@ int irq__exit(struct kvm *kvm)
 
 	free(irq_routing);
 
-	ent = rb_first(&pci_tree);
-	for (;;) {
+	while ((ent = rb_first(&pci_tree))) {
 		struct pci_dev *dev;
-		struct rb_node *next;
 		struct irq_line *line;
-		struct list_head *node, *tmp;
-
-		if (!ent)
-			break;
-
-		next = rb_next(ent);
 
 		dev = rb_entry(ent, struct pci_dev, node);
-		list_for_each_safe(node, tmp, &dev->lines) {
-			line = list_entry(node, struct irq_line, node);
+		while (!list_empty(&dev->lines)) {
+			line = list_first_entry(&dev->lines, struct irq_line, node);
+			list_del(&line->node);
 			free(line);
 		}
+		rb_erase(&dev->node, &pci_tree);
 		free(dev);
-		ent = next;
 	}
 
 	return 0;
-- 
1.7.8.5

[PATCH 2/4] kvm tools: use bitwise 'and' in kvm-ipc

[Sasha Levin]

From: Sasha Levin <levinsasha928@gmail.com>

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/kvm-ipc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/kvm-ipc.c b/tools/kvm/kvm-ipc.c
index 257c806c..70831b8 100644
--- a/tools/kvm/kvm-ipc.c
+++ b/tools/kvm/kvm-ipc.c
@@ -153,7 +153,7 @@ static void *kvm_ipc__thread(void *param)
 					r = kvm_ipc__receive(client);
 				} while	(r == 0);
 
-			} else if (event.events && (EPOLLERR | EPOLLRDHUP | EPOLLHUP)) {
+			} else if (event.events & (EPOLLERR | EPOLLRDHUP | EPOLLHUP)) {
 				kvm_ipc__close_conn(fd);
 			} else {
 				kvm_ipc__receive(fd);
-- 
1.7.8.5

[PATCH 3/4] kvm tools: use accessor function for virtio-9p FIDs

[Sasha Levin]

From: Sasha Levin <levinsasha928@gmail.com>

Since the 9p functions don't know the size of the fid array, they might
request an FID outside of the allowed range. Use an accessor to prevent
that and to hide the internal implementation from them.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/virtio/9p.c |   66 +++++++++++++++++++++++++++---------------------
 1 files changed, 37 insertions(+), 29 deletions(-)

diff --git a/tools/kvm/virtio/9p.c b/tools/kvm/virtio/9p.c
index e054dff..35e089d 100644
--- a/tools/kvm/virtio/9p.c
+++ b/tools/kvm/virtio/9p.c
@@ -22,6 +22,14 @@
 static LIST_HEAD(devs);
 static int compat_id = -1;
 
+static struct p9_fid *get_fid(struct p9_dev *p9dev, int fid)
+{
+	if (fid >= VIRTIO_9P_MAX_FID)
+		die("virtio-9p max FID (%u) reached!", VIRTIO_9P_MAX_FID);
+
+	return &p9dev->fids[fid];
+}
+
 /* Warning: Immediately use value returned from this function */
 static const char *rel_to_abs(struct p9_dev *p9dev,
 			      const char *path, char *abs_path)
@@ -156,7 +164,7 @@ static void virtio_p9_open(struct p9_dev *p9dev,
 
 
 	virtio_p9_pdu_readf(pdu, "dd", &fid, &flags);
-	new_fid = &p9dev->fids[fid];
+	new_fid = get_fid(p9dev, fid);
 
 	if (lstat(new_fid->abs_path, &st) < 0)
 		goto err_out;
@@ -197,7 +205,7 @@ static void virtio_p9_create(struct p9_dev *p9dev,
 
 	virtio_p9_pdu_readf(pdu, "dsddd", &dfid_val,
 			    &name, &flags, &mode, &gid);
-	dfid = &p9dev->fids[dfid_val];
+	dfid = get_fid(p9dev, dfid_val);
 
 	flags = virtio_p9_openflags(flags);
 
@@ -245,7 +253,7 @@ static void virtio_p9_mkdir(struct p9_dev *p9dev,
 
 	virtio_p9_pdu_readf(pdu, "dsdd", &dfid_val,
 			    &name, &mode, &gid);
-	dfid = &p9dev->fids[dfid_val];
+	dfid = get_fid(p9dev, dfid_val);
 
 	sprintf(full_path, "%s/%s", dfid->abs_path, name);
 	ret = mkdir(full_path, mode);
@@ -287,11 +295,11 @@ static void virtio_p9_walk(struct p9_dev *p9dev,
 
 
 	virtio_p9_pdu_readf(pdu, "ddw", &fid_val, &newfid_val, &nwname);
-	new_fid	= &p9dev->fids[newfid_val];
+	new_fid	= get_fid(p9dev, newfid_val);
 
 	nwqid = 0;
 	if (nwname) {
-		struct p9_fid *fid = &p9dev->fids[fid_val];
+		struct p9_fid *fid = get_fid(p9dev, fid_val);
 
 		strcpy(new_fid->path, fid->path);
 		/* skip the space for count */
@@ -366,7 +374,7 @@ static void virtio_p9_attach(struct p9_dev *p9dev,
 
 	stat2qid(&st, &qid);
 
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 	fid->fid = fid_val;
 	fid->uid = uid;
 	fid->is_dir = 1;
@@ -418,7 +426,7 @@ static void virtio_p9_read(struct p9_dev *p9dev,
 
 	rcount = 0;
 	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	iov_base = pdu->in_iov[0].iov_base;
 	iov_len  = pdu->in_iov[0].iov_len;
@@ -469,7 +477,7 @@ static void virtio_p9_readdir(struct p9_dev *p9dev,
 
 	rcount = 0;
 	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	if (!fid->is_dir) {
 		errno = -EINVAL;
@@ -525,7 +533,7 @@ static void virtio_p9_getattr(struct p9_dev *p9dev,
 	struct p9_stat_dotl statl;
 
 	virtio_p9_pdu_readf(pdu, "dq", &fid_val, &request_mask);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 	if (lstat(fid->abs_path, &st) < 0)
 		goto err_out;
 
@@ -573,7 +581,7 @@ static void virtio_p9_setattr(struct p9_dev *p9dev,
 	struct p9_iattr_dotl p9attr;
 
 	virtio_p9_pdu_readf(pdu, "dI", &fid_val, &p9attr);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	if (p9attr.valid & ATTR_MODE) {
 		ret = chmod(fid->abs_path, p9attr.mode);
@@ -652,7 +660,7 @@ static void virtio_p9_write(struct p9_dev *p9dev,
 	int twrite_size = sizeof(u32) + sizeof(u64) + sizeof(u32);
 
 	virtio_p9_pdu_readf(pdu, "dqd", &fid_val, &offset, &count);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	iov_base = pdu->out_iov[0].iov_base;
 	iov_len  = pdu->out_iov[0].iov_len;
@@ -691,7 +699,7 @@ static void virtio_p9_remove(struct p9_dev *p9dev,
 	struct p9_fid *fid;
 
 	virtio_p9_pdu_readf(pdu, "d", &fid_val);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	ret = remove(fid->abs_path);
 	if (ret < 0)
@@ -714,8 +722,8 @@ static void virtio_p9_rename(struct p9_dev *p9dev,
 	char full_path[PATH_MAX], *new_name;
 
 	virtio_p9_pdu_readf(pdu, "dds", &fid_val, &new_fid_val, &new_name);
-	fid = &p9dev->fids[fid_val];
-	new_fid = &p9dev->fids[new_fid_val];
+	fid = get_fid(p9dev, fid_val);
+	new_fid = get_fid(p9dev, new_fid_val);
 
 	sprintf(full_path, "%s/%s", new_fid->abs_path, new_name);
 	ret = rename(fid->abs_path, full_path);
@@ -740,7 +748,7 @@ static void virtio_p9_readlink(struct p9_dev *p9dev,
 	char target_path[PATH_MAX];
 
 	virtio_p9_pdu_readf(pdu, "d", &fid_val);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	memset(target_path, 0, PATH_MAX);
 	ret = readlink(fid->abs_path, target_path, PATH_MAX - 1);
@@ -766,7 +774,7 @@ static void virtio_p9_statfs(struct p9_dev *p9dev,
 	struct statfs stat_buf;
 
 	virtio_p9_pdu_readf(pdu, "d", &fid_val);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	ret = statfs(fid->abs_path, &stat_buf);
 	if (ret < 0)
@@ -801,7 +809,7 @@ static void virtio_p9_mknod(struct p9_dev *p9dev,
 	virtio_p9_pdu_readf(pdu, "dsdddd", &fid_val, &name, &mode,
 			    &major, &minor, &gid);
 
-	dfid = &p9dev->fids[fid_val];
+	dfid = get_fid(p9dev, fid_val);
 	sprintf(full_path, "%s/%s", dfid->abs_path, name);
 	ret = mknod(full_path, mode, makedev(major, minor));
 	if (ret < 0)
@@ -838,7 +846,7 @@ static void virtio_p9_fsync(struct p9_dev *p9dev,
 	u32 fid_val, datasync;
 
 	virtio_p9_pdu_readf(pdu, "dd", &fid_val, &datasync);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	if (datasync)
 		ret = fdatasync(fid->fd);
@@ -867,7 +875,7 @@ static void virtio_p9_symlink(struct p9_dev *p9dev,
 
 	virtio_p9_pdu_readf(pdu, "dssd", &fid_val, &name, &old_path, &gid);
 
-	dfid = &p9dev->fids[fid_val];
+	dfid = get_fid(p9dev, fid_val);
 	sprintf(new_name, "%s/%s", dfid->abs_path, name);
 	ret = symlink(old_path, new_name);
 	if (ret < 0)
@@ -905,8 +913,8 @@ static void virtio_p9_link(struct p9_dev *p9dev,
 
 	virtio_p9_pdu_readf(pdu, "dds", &dfid_val, &fid_val, &name);
 
-	dfid = &p9dev->fids[dfid_val];
-	fid =  &p9dev->fids[fid_val];
+	dfid = get_fid(p9dev, dfid_val);
+	fid =  get_fid(p9dev, fid_val);
 	sprintf(full_path, "%s/%s", dfid->abs_path, name);
 	ret = link(fid->abs_path, full_path);
 	if (ret < 0)
@@ -1006,8 +1014,8 @@ static void virtio_p9_renameat(struct p9_dev *p9dev,
 	virtio_p9_pdu_readf(pdu, "dsds", &old_dfid_val, &old_name,
 			    &new_dfid_val, &new_name);
 
-	old_dfid = &p9dev->fids[old_dfid_val];
-	new_dfid = &p9dev->fids[new_dfid_val];
+	old_dfid = get_fid(p9dev, old_dfid_val);
+	new_dfid = get_fid(p9dev, new_dfid_val);
 
 	sprintf(old_full_path, "%s/%s", old_dfid->abs_path, old_name);
 	sprintf(new_full_path, "%s/%s", new_dfid->abs_path, new_name);
@@ -1019,9 +1027,9 @@ static void virtio_p9_renameat(struct p9_dev *p9dev,
 	 * that.
 	 */
 	for (i = 0; i < VIRTIO_9P_MAX_FID; i++) {
-		if (p9dev->fids[i].fid != P9_NOFID &&
-		    virtio_p9_ancestor(p9dev->fids[i].path, old_name)) {
-			virtio_p9_fix_path(p9dev->fids[i].path, old_name,
+		if (get_fid(p9dev, i)->fid != P9_NOFID &&
+		    virtio_p9_ancestor(get_fid(p9dev, i)->path, old_name)) {
+			virtio_p9_fix_path(get_fid(p9dev, i)->path, old_name,
 					   new_name);
 		}
 	}
@@ -1047,7 +1055,7 @@ static void virtio_p9_unlinkat(struct p9_dev *p9dev,
 	char full_path[PATH_MAX];
 
 	virtio_p9_pdu_readf(pdu, "dsd", &fid_val, &name, &flags);
-	fid = &p9dev->fids[fid_val];
+	fid = get_fid(p9dev, fid_val);
 
 	sprintf(full_path, "%s/%s", fid->abs_path, name);
 	ret = remove(full_path);
@@ -1293,8 +1301,8 @@ int virtio_9p__register(struct kvm *kvm, const char *root, const char *tag_name)
 	 * absolute path of an fid without playing with strings.
 	 */
 	for (i = 0; i < VIRTIO_9P_MAX_FID; i++) {
-		strcpy(p9dev->fids[i].abs_path, root);
-		p9dev->fids[i].path = p9dev->fids[i].abs_path + root_len;
+		strcpy(get_fid(p9dev, i)->abs_path, root);
+		get_fid(p9dev, i)->path = get_fid(p9dev, i)->abs_path + root_len;
 	}
 	p9dev->config->tag_len = strlen(tag_name);
 	if (p9dev->config->tag_len > MAX_TAG_LEN) {
-- 
1.7.8.5

[PATCH 4/4] kvm tools: increase amount of virtio-9p FIDs

[Sasha Levin]

From: Sasha Levin <levinsasha928@gmail.com>

Increase the amount of FIDs since we easily reach current amount with
simple stress tests.

This should be changed to use a rbtree or something faster in the
future.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 tools/kvm/include/kvm/virtio-9p.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-9p.h b/tools/kvm/include/kvm/virtio-9p.h
index cd2869a..5902701 100644
--- a/tools/kvm/include/kvm/virtio-9p.h
+++ b/tools/kvm/include/kvm/virtio-9p.h
@@ -12,7 +12,7 @@
 #define VIRTQUEUE_NUM		128
 #define	VIRTIO_9P_DEFAULT_TAG	"kvm_9p"
 #define VIRTIO_9P_HDR_LEN	(sizeof(u32)+sizeof(u8)+sizeof(u16))
-#define VIRTIO_9P_MAX_FID	256
+#define VIRTIO_9P_MAX_FID	16384
 #define VIRTIO_9P_VERSION_DOTL	"9P2000.L"
 #define MAX_TAG_LEN		32
 
-- 
1.7.8.5

Re: [PATCH 1/4] kvm tools: fix sigsegv in irq__exit

[Pekka Enberg]

On Wed, May 9, 2012 at 3:24 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> From: Sasha Levin <levinsasha928@gmail.com>
>
> We free the structure, but never remove them from the tree or list, then
> we freed them the next time we ran through that structure.
>
> This patch also simplifies irq__exit a bit.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

Applied all patches. Thanks!