* [Qemu-devel] [PULL 01/12] 9pfs: allocate space for guest originated empty strings
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 02/12] 9pfs: fix potential host memory leak in v9fs_read Greg Kurz
` (11 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz, Li Qiang
From: Li Qiang <liqiang6-s@360.cn>
If a guest sends an empty string paramater to any 9P operation, the current
code unmarshals it into a V9fsString equal to { .size = 0, .data = NULL }.
This is unfortunate because it can cause NULL pointer dereference to happen
at various locations in the 9pfs code. And we don't want to check str->data
everywhere we pass it to strcmp() or any other function which expects a
dereferenceable pointer.
This patch enforces the allocation of genuine C empty strings instead, so
callers don't have to bother.
Out of all v9fs_iov_vunmarshal() users, only v9fs_xattrwalk() checks if
the returned string is empty. It now uses v9fs_string_size() since
name.data cannot be NULL anymore.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
[groug, rewritten title and changelog,
fix empty string check in v9fs_xattrwalk()]
Signed-off-by: Greg Kurz <groug@kaod.org>
---
fsdev/9p-iov-marshal.c | 2 +-
hw/9pfs/9p.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fsdev/9p-iov-marshal.c b/fsdev/9p-iov-marshal.c
index 663cad542900..1d16f8df4bd4 100644
--- a/fsdev/9p-iov-marshal.c
+++ b/fsdev/9p-iov-marshal.c
@@ -125,7 +125,7 @@ ssize_t v9fs_iov_vunmarshal(struct iovec *out_sg, int out_num, size_t offset,
str->data = g_malloc(str->size + 1);
copied = v9fs_unpack(str->data, out_sg, out_num, offset,
str->size);
- if (copied > 0) {
+ if (copied >= 0) {
str->data[str->size] = 0;
} else {
v9fs_string_free(str);
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 119ee584969b..39a7e1d52d2a 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3174,7 +3174,7 @@ static void v9fs_xattrwalk(void *opaque)
goto out;
}
v9fs_path_copy(&xattr_fidp->path, &file_fidp->path);
- if (name.data == NULL) {
+ if (!v9fs_string_size(&name)) {
/*
* listxattr request. Get the size first
*/
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 02/12] 9pfs: fix potential host memory leak in v9fs_read
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 01/12] 9pfs: allocate space for guest originated empty strings Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 03/12] 9pfs: fsdev: drop useless extern annotation for functions Greg Kurz
` (10 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz, Li Qiang
From: Li Qiang <liqiang6-s@360.cn>
In 9pfs read dispatch function, it doesn't free two QEMUIOVector
object thus causing potential memory leak. This patch avoid this.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 39a7e1d52d2a..ff94a6272c3c 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1826,14 +1826,15 @@ static void v9fs_read(void *opaque)
if (len < 0) {
/* IO error return the error */
err = len;
- goto out;
+ goto out_free_iovec;
}
} while (count < max_count && len > 0);
err = pdu_marshal(pdu, offset, "d", count);
if (err < 0) {
- goto out;
+ goto out_free_iovec;
}
err += offset + count;
+out_free_iovec:
qemu_iovec_destroy(&qiov);
qemu_iovec_destroy(&qiov_full);
} else if (fidp->fid_type == P9_FID_XATTR) {
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 03/12] 9pfs: fsdev: drop useless extern annotation for functions
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 01/12] 9pfs: allocate space for guest originated empty strings Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 02/12] 9pfs: fix potential host memory leak in v9fs_read Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 04/12] 9pfs: use coroutine_fn annotation in hw/9pfs/co*.[ch] Greg Kurz
` (9 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz
Signed-off-by: Greg Kurz <groug@kaod.org>
---
fsdev/9p-marshal.h | 6 ++--
hw/9pfs/9p-synth.h | 10 +++---
hw/9pfs/9p.h | 18 +++++-----
hw/9pfs/coth.h | 94 ++++++++++++++++++++++++++---------------------------
hw/9pfs/virtio-9p.h | 2 +-
5 files changed, 65 insertions(+), 65 deletions(-)
diff --git a/fsdev/9p-marshal.h b/fsdev/9p-marshal.h
index 77f7fef326ee..c8823d878f98 100644
--- a/fsdev/9p-marshal.h
+++ b/fsdev/9p-marshal.h
@@ -76,8 +76,8 @@ static inline void v9fs_string_init(V9fsString *str)
str->data = NULL;
str->size = 0;
}
-extern void v9fs_string_free(V9fsString *str);
-extern void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
-extern void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
+void v9fs_string_free(V9fsString *str);
+void v9fs_string_sprintf(V9fsString *str, const char *fmt, ...);
+void v9fs_string_copy(V9fsString *lhs, V9fsString *rhs);
#endif
diff --git a/hw/9pfs/9p-synth.h b/hw/9pfs/9p-synth.h
index 6bcb44ace230..49c2fc7b274e 100644
--- a/hw/9pfs/9p-synth.h
+++ b/hw/9pfs/9p-synth.h
@@ -43,10 +43,10 @@ typedef struct V9fsSynthOpenState {
struct dirent dent;
} V9fsSynthOpenState;
-extern int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
- const char *name, V9fsSynthNode **result);
-extern int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
- const char *name, v9fs_synth_read read,
- v9fs_synth_write write, void *arg);
+int qemu_v9fs_synth_mkdir(V9fsSynthNode *parent, int mode,
+ const char *name, V9fsSynthNode **result);
+int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int mode,
+ const char *name, v9fs_synth_read read,
+ v9fs_synth_write write, void *arg);
#endif
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index d539d2ebe9c0..5225b4f12071 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -324,15 +324,15 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
return pdu->cancelled;
}
-extern void v9fs_reclaim_fd(V9fsPDU *pdu);
-extern void v9fs_path_init(V9fsPath *path);
-extern void v9fs_path_free(V9fsPath *path);
-extern void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
-extern void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
-extern int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
- const char *name, V9fsPath *path);
-extern int v9fs_device_realize_common(V9fsState *s, Error **errp);
-extern void v9fs_device_unrealize_common(V9fsState *s, Error **errp);
+void v9fs_reclaim_fd(V9fsPDU *pdu);
+void v9fs_path_init(V9fsPath *path);
+void v9fs_path_free(V9fsPath *path);
+void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
+void v9fs_path_copy(V9fsPath *lhs, V9fsPath *rhs);
+int v9fs_name_to_path(V9fsState *s, V9fsPath *dirpath,
+ const char *name, V9fsPath *path);
+int v9fs_device_realize_common(V9fsState *s, Error **errp);
+void v9fs_device_unrealize_common(V9fsState *s, Error **errp);
ssize_t pdu_marshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...);
ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...);
diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
index 3c7424e423d9..af6db5e84e34 100644
--- a/hw/9pfs/coth.h
+++ b/hw/9pfs/coth.h
@@ -47,52 +47,52 @@
qemu_coroutine_yield(); \
} while (0)
-extern void co_run_in_worker_bh(void *);
-extern int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *);
-extern int v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **);
-extern off_t v9fs_co_telldir(V9fsPDU *, V9fsFidState *);
-extern void v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t);
-extern void v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *);
-extern int v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *);
-extern int v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *);
-extern int v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t);
-extern int v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]);
-extern int v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t);
-extern int v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t);
-extern int v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
-extern int v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
- V9fsString *, void *, size_t);
-extern int v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t,
- gid_t, dev_t, mode_t, struct stat *);
-extern int v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *,
- mode_t, uid_t, gid_t, struct stat *);
-extern int v9fs_co_remove(V9fsPDU *, V9fsPath *);
-extern int v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *);
-extern int v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, int flags);
-extern int v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *,
- V9fsPath *, V9fsString *);
-extern int v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *);
-extern int v9fs_co_opendir(V9fsPDU *, V9fsFidState *);
-extern int v9fs_co_open(V9fsPDU *, V9fsFidState *, int);
-extern int v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *,
- gid_t, int, int, struct stat *);
-extern int v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *,
- void *, size_t, int);
-extern int v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *);
-extern int v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *);
-extern int v9fs_co_close(V9fsPDU *, V9fsFidOpenState *);
-extern int v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int);
-extern int v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *,
- const char *, gid_t, struct stat *);
-extern int v9fs_co_link(V9fsPDU *, V9fsFidState *,
- V9fsFidState *, V9fsString *);
-extern int v9fs_co_pwritev(V9fsPDU *, V9fsFidState *,
- struct iovec *, int, int64_t);
-extern int v9fs_co_preadv(V9fsPDU *, V9fsFidState *,
- struct iovec *, int, int64_t);
-extern int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *,
- const char *, V9fsPath *);
-extern int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t,
- V9fsStatDotl *v9stat);
+void co_run_in_worker_bh(void *);
+int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *);
+int v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **);
+off_t v9fs_co_telldir(V9fsPDU *, V9fsFidState *);
+void v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t);
+void v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *);
+int v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *);
+int v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *);
+int v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t);
+int v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]);
+int v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t);
+int v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t);
+int v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
+int v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
+ V9fsString *, void *, size_t);
+int v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t,
+ gid_t, dev_t, mode_t, struct stat *);
+int v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *,
+ mode_t, uid_t, gid_t, struct stat *);
+int v9fs_co_remove(V9fsPDU *, V9fsPath *);
+int v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *);
+int v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, int flags);
+int v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *,
+ V9fsPath *, V9fsString *);
+int v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *);
+int v9fs_co_opendir(V9fsPDU *, V9fsFidState *);
+int v9fs_co_open(V9fsPDU *, V9fsFidState *, int);
+int v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *,
+ gid_t, int, int, struct stat *);
+int v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *,
+ void *, size_t, int);
+int v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *);
+int v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *);
+int v9fs_co_close(V9fsPDU *, V9fsFidOpenState *);
+int v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int);
+int v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *,
+ const char *, gid_t, struct stat *);
+int v9fs_co_link(V9fsPDU *, V9fsFidState *,
+ V9fsFidState *, V9fsString *);
+int v9fs_co_pwritev(V9fsPDU *, V9fsFidState *,
+ struct iovec *, int, int64_t);
+int v9fs_co_preadv(V9fsPDU *, V9fsFidState *,
+ struct iovec *, int, int64_t);
+int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *,
+ const char *, V9fsPath *);
+int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t,
+ V9fsStatDotl *v9stat);
#endif
diff --git a/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 7586b792d61e..25c47c7cb6d6 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -15,7 +15,7 @@ typedef struct V9fsVirtioState
V9fsState state;
} V9fsVirtioState;
-extern void virtio_9p_push_and_notify(V9fsPDU *pdu);
+void virtio_9p_push_and_notify(V9fsPDU *pdu);
ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
const char *fmt, va_list ap);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 04/12] 9pfs: use coroutine_fn annotation in hw/9pfs/co*.[ch]
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (2 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 03/12] 9pfs: fsdev: drop useless extern annotation for functions Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 05/12] 9pfs: use coroutine_fn annotation in hw/9pfs/9p.[ch] Greg Kurz
` (8 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz
All these functions use the v9fs_co_run_in_worker() macro, and thus always
call qemu_coroutine_self() and qemu_coroutine_yield().
Let's mark them to make it obvious they execute in coroutine context.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/codir.c | 17 +++++-----
hw/9pfs/cofile.c | 32 ++++++++++---------
hw/9pfs/cofs.c | 43 ++++++++++++++-----------
hw/9pfs/coth.h | 93 ++++++++++++++++++++++++++++---------------------------
hw/9pfs/coxattr.c | 19 ++++++------
5 files changed, 109 insertions(+), 95 deletions(-)
diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index d91f9ad6eb9e..7cd6fce1ad37 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -17,7 +17,8 @@
#include "qemu/coroutine.h"
#include "coth.h"
-int v9fs_co_readdir(V9fsPDU *pdu, V9fsFidState *fidp, struct dirent **dent)
+int coroutine_fn v9fs_co_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
+ struct dirent **dent)
{
int err;
V9fsState *s = pdu->s;
@@ -59,7 +60,8 @@ off_t v9fs_co_telldir(V9fsPDU *pdu, V9fsFidState *fidp)
return err;
}
-void v9fs_co_seekdir(V9fsPDU *pdu, V9fsFidState *fidp, off_t offset)
+void coroutine_fn v9fs_co_seekdir(V9fsPDU *pdu, V9fsFidState *fidp,
+ off_t offset)
{
V9fsState *s = pdu->s;
if (v9fs_request_cancelled(pdu)) {
@@ -71,7 +73,7 @@ void v9fs_co_seekdir(V9fsPDU *pdu, V9fsFidState *fidp, off_t offset)
});
}
-void v9fs_co_rewinddir(V9fsPDU *pdu, V9fsFidState *fidp)
+void coroutine_fn v9fs_co_rewinddir(V9fsPDU *pdu, V9fsFidState *fidp)
{
V9fsState *s = pdu->s;
if (v9fs_request_cancelled(pdu)) {
@@ -83,8 +85,9 @@ void v9fs_co_rewinddir(V9fsPDU *pdu, V9fsFidState *fidp)
});
}
-int v9fs_co_mkdir(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name,
- mode_t mode, uid_t uid, gid_t gid, struct stat *stbuf)
+int coroutine_fn v9fs_co_mkdir(V9fsPDU *pdu, V9fsFidState *fidp,
+ V9fsString *name, mode_t mode, uid_t uid,
+ gid_t gid, struct stat *stbuf)
{
int err;
FsCred cred;
@@ -120,7 +123,7 @@ int v9fs_co_mkdir(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name,
return err;
}
-int v9fs_co_opendir(V9fsPDU *pdu, V9fsFidState *fidp)
+int coroutine_fn v9fs_co_opendir(V9fsPDU *pdu, V9fsFidState *fidp)
{
int err;
V9fsState *s = pdu->s;
@@ -148,7 +151,7 @@ int v9fs_co_opendir(V9fsPDU *pdu, V9fsFidState *fidp)
return err;
}
-int v9fs_co_closedir(V9fsPDU *pdu, V9fsFidOpenState *fs)
+int coroutine_fn v9fs_co_closedir(V9fsPDU *pdu, V9fsFidOpenState *fs)
{
int err;
V9fsState *s = pdu->s;
diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 10343c0a93b4..120e2671080b 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -17,8 +17,8 @@
#include "qemu/coroutine.h"
#include "coth.h"
-int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
- V9fsStatDotl *v9stat)
+int coroutine_fn v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
+ V9fsStatDotl *v9stat)
{
int err = 0;
V9fsState *s = pdu->s;
@@ -41,7 +41,7 @@ int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t st_mode,
return err;
}
-int v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf)
+int coroutine_fn v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf)
{
int err;
V9fsState *s = pdu->s;
@@ -61,7 +61,8 @@ int v9fs_co_lstat(V9fsPDU *pdu, V9fsPath *path, struct stat *stbuf)
return err;
}
-int v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp, struct stat *stbuf)
+int coroutine_fn v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp,
+ struct stat *stbuf)
{
int err;
V9fsState *s = pdu->s;
@@ -93,7 +94,7 @@ int v9fs_co_fstat(V9fsPDU *pdu, V9fsFidState *fidp, struct stat *stbuf)
return err;
}
-int v9fs_co_open(V9fsPDU *pdu, V9fsFidState *fidp, int flags)
+int coroutine_fn v9fs_co_open(V9fsPDU *pdu, V9fsFidState *fidp, int flags)
{
int err;
V9fsState *s = pdu->s;
@@ -121,8 +122,9 @@ int v9fs_co_open(V9fsPDU *pdu, V9fsFidState *fidp, int flags)
return err;
}
-int v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, gid_t gid,
- int flags, int mode, struct stat *stbuf)
+int coroutine_fn v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp,
+ V9fsString *name, gid_t gid, int flags, int mode,
+ struct stat *stbuf)
{
int err;
FsCred cred;
@@ -175,7 +177,7 @@ int v9fs_co_open2(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, gid_t gid,
return err;
}
-int v9fs_co_close(V9fsPDU *pdu, V9fsFidOpenState *fs)
+int coroutine_fn v9fs_co_close(V9fsPDU *pdu, V9fsFidOpenState *fs)
{
int err;
V9fsState *s = pdu->s;
@@ -196,7 +198,7 @@ int v9fs_co_close(V9fsPDU *pdu, V9fsFidOpenState *fs)
return err;
}
-int v9fs_co_fsync(V9fsPDU *pdu, V9fsFidState *fidp, int datasync)
+int coroutine_fn v9fs_co_fsync(V9fsPDU *pdu, V9fsFidState *fidp, int datasync)
{
int err;
V9fsState *s = pdu->s;
@@ -214,8 +216,8 @@ int v9fs_co_fsync(V9fsPDU *pdu, V9fsFidState *fidp, int datasync)
return err;
}
-int v9fs_co_link(V9fsPDU *pdu, V9fsFidState *oldfid,
- V9fsFidState *newdirfid, V9fsString *name)
+int coroutine_fn v9fs_co_link(V9fsPDU *pdu, V9fsFidState *oldfid,
+ V9fsFidState *newdirfid, V9fsString *name)
{
int err;
V9fsState *s = pdu->s;
@@ -236,8 +238,8 @@ int v9fs_co_link(V9fsPDU *pdu, V9fsFidState *oldfid,
return err;
}
-int v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
- struct iovec *iov, int iovcnt, int64_t offset)
+int coroutine_fn v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
+ struct iovec *iov, int iovcnt, int64_t offset)
{
int err;
V9fsState *s = pdu->s;
@@ -255,8 +257,8 @@ int v9fs_co_pwritev(V9fsPDU *pdu, V9fsFidState *fidp,
return err;
}
-int v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
- struct iovec *iov, int iovcnt, int64_t offset)
+int coroutine_fn v9fs_co_preadv(V9fsPDU *pdu, V9fsFidState *fidp,
+ struct iovec *iov, int iovcnt, int64_t offset)
{
int err;
V9fsState *s = pdu->s;
diff --git a/hw/9pfs/cofs.c b/hw/9pfs/cofs.c
index 70f584fcbd21..c62103221d2a 100644
--- a/hw/9pfs/cofs.c
+++ b/hw/9pfs/cofs.c
@@ -49,7 +49,7 @@ static ssize_t __readlink(V9fsState *s, V9fsPath *path, V9fsString *buf)
return len;
}
-int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf)
+int coroutine_fn v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf)
{
int err;
V9fsState *s = pdu->s;
@@ -69,7 +69,8 @@ int v9fs_co_readlink(V9fsPDU *pdu, V9fsPath *path, V9fsString *buf)
return err;
}
-int v9fs_co_statfs(V9fsPDU *pdu, V9fsPath *path, struct statfs *stbuf)
+int coroutine_fn v9fs_co_statfs(V9fsPDU *pdu, V9fsPath *path,
+ struct statfs *stbuf)
{
int err;
V9fsState *s = pdu->s;
@@ -89,7 +90,7 @@ int v9fs_co_statfs(V9fsPDU *pdu, V9fsPath *path, struct statfs *stbuf)
return err;
}
-int v9fs_co_chmod(V9fsPDU *pdu, V9fsPath *path, mode_t mode)
+int coroutine_fn v9fs_co_chmod(V9fsPDU *pdu, V9fsPath *path, mode_t mode)
{
int err;
FsCred cred;
@@ -112,8 +113,8 @@ int v9fs_co_chmod(V9fsPDU *pdu, V9fsPath *path, mode_t mode)
return err;
}
-int v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path,
- struct timespec times[2])
+int coroutine_fn v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path,
+ struct timespec times[2])
{
int err;
V9fsState *s = pdu->s;
@@ -133,7 +134,8 @@ int v9fs_co_utimensat(V9fsPDU *pdu, V9fsPath *path,
return err;
}
-int v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid, gid_t gid)
+int coroutine_fn v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid,
+ gid_t gid)
{
int err;
FsCred cred;
@@ -157,7 +159,7 @@ int v9fs_co_chown(V9fsPDU *pdu, V9fsPath *path, uid_t uid, gid_t gid)
return err;
}
-int v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size)
+int coroutine_fn v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size)
{
int err;
V9fsState *s = pdu->s;
@@ -177,8 +179,9 @@ int v9fs_co_truncate(V9fsPDU *pdu, V9fsPath *path, off_t size)
return err;
}
-int v9fs_co_mknod(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, uid_t uid,
- gid_t gid, dev_t dev, mode_t mode, struct stat *stbuf)
+int coroutine_fn v9fs_co_mknod(V9fsPDU *pdu, V9fsFidState *fidp,
+ V9fsString *name, uid_t uid, gid_t gid,
+ dev_t dev, mode_t mode, struct stat *stbuf)
{
int err;
V9fsPath path;
@@ -216,7 +219,7 @@ int v9fs_co_mknod(V9fsPDU *pdu, V9fsFidState *fidp, V9fsString *name, uid_t uid,
}
/* Only works with path name based fid */
-int v9fs_co_remove(V9fsPDU *pdu, V9fsPath *path)
+int coroutine_fn v9fs_co_remove(V9fsPDU *pdu, V9fsPath *path)
{
int err;
V9fsState *s = pdu->s;
@@ -236,7 +239,8 @@ int v9fs_co_remove(V9fsPDU *pdu, V9fsPath *path)
return err;
}
-int v9fs_co_unlinkat(V9fsPDU *pdu, V9fsPath *path, V9fsString *name, int flags)
+int coroutine_fn v9fs_co_unlinkat(V9fsPDU *pdu, V9fsPath *path,
+ V9fsString *name, int flags)
{
int err;
V9fsState *s = pdu->s;
@@ -257,7 +261,8 @@ int v9fs_co_unlinkat(V9fsPDU *pdu, V9fsPath *path, V9fsString *name, int flags)
}
/* Only work with path name based fid */
-int v9fs_co_rename(V9fsPDU *pdu, V9fsPath *oldpath, V9fsPath *newpath)
+int coroutine_fn v9fs_co_rename(V9fsPDU *pdu, V9fsPath *oldpath,
+ V9fsPath *newpath)
{
int err;
V9fsState *s = pdu->s;
@@ -275,8 +280,9 @@ int v9fs_co_rename(V9fsPDU *pdu, V9fsPath *oldpath, V9fsPath *newpath)
return err;
}
-int v9fs_co_renameat(V9fsPDU *pdu, V9fsPath *olddirpath, V9fsString *oldname,
- V9fsPath *newdirpath, V9fsString *newname)
+int coroutine_fn v9fs_co_renameat(V9fsPDU *pdu, V9fsPath *olddirpath,
+ V9fsString *oldname, V9fsPath *newdirpath,
+ V9fsString *newname)
{
int err;
V9fsState *s = pdu->s;
@@ -295,8 +301,9 @@ int v9fs_co_renameat(V9fsPDU *pdu, V9fsPath *olddirpath, V9fsString *oldname,
return err;
}
-int v9fs_co_symlink(V9fsPDU *pdu, V9fsFidState *dfidp, V9fsString *name,
- const char *oldpath, gid_t gid, struct stat *stbuf)
+int coroutine_fn v9fs_co_symlink(V9fsPDU *pdu, V9fsFidState *dfidp,
+ V9fsString *name, const char *oldpath,
+ gid_t gid, struct stat *stbuf)
{
int err;
FsCred cred;
@@ -337,8 +344,8 @@ int v9fs_co_symlink(V9fsPDU *pdu, V9fsFidState *dfidp, V9fsString *name,
* For path name based fid we don't block. So we can
* directly call the fs driver ops.
*/
-int v9fs_co_name_to_path(V9fsPDU *pdu, V9fsPath *dirpath,
- const char *name, V9fsPath *path)
+int coroutine_fn v9fs_co_name_to_path(V9fsPDU *pdu, V9fsPath *dirpath,
+ const char *name, V9fsPath *path)
{
int err;
V9fsState *s = pdu->s;
diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
index af6db5e84e34..19e4d9287eef 100644
--- a/hw/9pfs/coth.h
+++ b/hw/9pfs/coth.h
@@ -48,51 +48,52 @@
} while (0)
void co_run_in_worker_bh(void *);
-int v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *);
-int v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **);
-off_t v9fs_co_telldir(V9fsPDU *, V9fsFidState *);
-void v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t);
-void v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *);
-int v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *);
-int v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *);
-int v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t);
-int v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]);
-int v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t);
-int v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t);
-int v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
-int v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
- V9fsString *, void *, size_t);
-int v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t,
- gid_t, dev_t, mode_t, struct stat *);
-int v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *,
- mode_t, uid_t, gid_t, struct stat *);
-int v9fs_co_remove(V9fsPDU *, V9fsPath *);
-int v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *);
-int v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *, int flags);
-int v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *,
- V9fsPath *, V9fsString *);
-int v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *);
-int v9fs_co_opendir(V9fsPDU *, V9fsFidState *);
-int v9fs_co_open(V9fsPDU *, V9fsFidState *, int);
-int v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *,
- gid_t, int, int, struct stat *);
-int v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *,
- void *, size_t, int);
-int v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *);
-int v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *);
-int v9fs_co_close(V9fsPDU *, V9fsFidOpenState *);
-int v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int);
-int v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *,
- const char *, gid_t, struct stat *);
-int v9fs_co_link(V9fsPDU *, V9fsFidState *,
- V9fsFidState *, V9fsString *);
-int v9fs_co_pwritev(V9fsPDU *, V9fsFidState *,
- struct iovec *, int, int64_t);
-int v9fs_co_preadv(V9fsPDU *, V9fsFidState *,
- struct iovec *, int, int64_t);
-int v9fs_co_name_to_path(V9fsPDU *, V9fsPath *,
- const char *, V9fsPath *);
-int v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t,
- V9fsStatDotl *v9stat);
+int coroutine_fn v9fs_co_readlink(V9fsPDU *, V9fsPath *, V9fsString *);
+int coroutine_fn v9fs_co_readdir(V9fsPDU *, V9fsFidState *, struct dirent **);
+off_t coroutine_fn v9fs_co_telldir(V9fsPDU *, V9fsFidState *);
+void coroutine_fn v9fs_co_seekdir(V9fsPDU *, V9fsFidState *, off_t);
+void coroutine_fn v9fs_co_rewinddir(V9fsPDU *, V9fsFidState *);
+int coroutine_fn v9fs_co_statfs(V9fsPDU *, V9fsPath *, struct statfs *);
+int coroutine_fn v9fs_co_lstat(V9fsPDU *, V9fsPath *, struct stat *);
+int coroutine_fn v9fs_co_chmod(V9fsPDU *, V9fsPath *, mode_t);
+int coroutine_fn v9fs_co_utimensat(V9fsPDU *, V9fsPath *, struct timespec [2]);
+int coroutine_fn v9fs_co_chown(V9fsPDU *, V9fsPath *, uid_t, gid_t);
+int coroutine_fn v9fs_co_truncate(V9fsPDU *, V9fsPath *, off_t);
+int coroutine_fn v9fs_co_llistxattr(V9fsPDU *, V9fsPath *, void *, size_t);
+int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *, V9fsPath *,
+ V9fsString *, void *, size_t);
+int coroutine_fn v9fs_co_mknod(V9fsPDU *, V9fsFidState *, V9fsString *, uid_t,
+ gid_t, dev_t, mode_t, struct stat *);
+int coroutine_fn v9fs_co_mkdir(V9fsPDU *, V9fsFidState *, V9fsString *,
+ mode_t, uid_t, gid_t, struct stat *);
+int coroutine_fn v9fs_co_remove(V9fsPDU *, V9fsPath *);
+int coroutine_fn v9fs_co_rename(V9fsPDU *, V9fsPath *, V9fsPath *);
+int coroutine_fn v9fs_co_unlinkat(V9fsPDU *, V9fsPath *, V9fsString *,
+ int flags);
+int coroutine_fn v9fs_co_renameat(V9fsPDU *, V9fsPath *, V9fsString *,
+ V9fsPath *, V9fsString *);
+int coroutine_fn v9fs_co_fstat(V9fsPDU *, V9fsFidState *, struct stat *);
+int coroutine_fn v9fs_co_opendir(V9fsPDU *, V9fsFidState *);
+int coroutine_fn v9fs_co_open(V9fsPDU *, V9fsFidState *, int);
+int coroutine_fn v9fs_co_open2(V9fsPDU *, V9fsFidState *, V9fsString *,
+ gid_t, int, int, struct stat *);
+int coroutine_fn v9fs_co_lsetxattr(V9fsPDU *, V9fsPath *, V9fsString *,
+ void *, size_t, int);
+int coroutine_fn v9fs_co_lremovexattr(V9fsPDU *, V9fsPath *, V9fsString *);
+int coroutine_fn v9fs_co_closedir(V9fsPDU *, V9fsFidOpenState *);
+int coroutine_fn v9fs_co_close(V9fsPDU *, V9fsFidOpenState *);
+int coroutine_fn v9fs_co_fsync(V9fsPDU *, V9fsFidState *, int);
+int coroutine_fn v9fs_co_symlink(V9fsPDU *, V9fsFidState *, V9fsString *,
+ const char *, gid_t, struct stat *);
+int coroutine_fn v9fs_co_link(V9fsPDU *, V9fsFidState *,
+ V9fsFidState *, V9fsString *);
+int coroutine_fn v9fs_co_pwritev(V9fsPDU *, V9fsFidState *,
+ struct iovec *, int, int64_t);
+int coroutine_fn v9fs_co_preadv(V9fsPDU *, V9fsFidState *,
+ struct iovec *, int, int64_t);
+int coroutine_fn v9fs_co_name_to_path(V9fsPDU *, V9fsPath *,
+ const char *, V9fsPath *);
+int coroutine_fn v9fs_co_st_gen(V9fsPDU *pdu, V9fsPath *path, mode_t,
+ V9fsStatDotl *v9stat);
#endif
diff --git a/hw/9pfs/coxattr.c b/hw/9pfs/coxattr.c
index 133c4ead37b6..154392eade73 100644
--- a/hw/9pfs/coxattr.c
+++ b/hw/9pfs/coxattr.c
@@ -17,7 +17,8 @@
#include "qemu/coroutine.h"
#include "coth.h"
-int v9fs_co_llistxattr(V9fsPDU *pdu, V9fsPath *path, void *value, size_t size)
+int coroutine_fn v9fs_co_llistxattr(V9fsPDU *pdu, V9fsPath *path, void *value,
+ size_t size)
{
int err;
V9fsState *s = pdu->s;
@@ -37,9 +38,9 @@ int v9fs_co_llistxattr(V9fsPDU *pdu, V9fsPath *path, void *value, size_t size)
return err;
}
-int v9fs_co_lgetxattr(V9fsPDU *pdu, V9fsPath *path,
- V9fsString *xattr_name,
- void *value, size_t size)
+int coroutine_fn v9fs_co_lgetxattr(V9fsPDU *pdu, V9fsPath *path,
+ V9fsString *xattr_name, void *value,
+ size_t size)
{
int err;
V9fsState *s = pdu->s;
@@ -61,9 +62,9 @@ int v9fs_co_lgetxattr(V9fsPDU *pdu, V9fsPath *path,
return err;
}
-int v9fs_co_lsetxattr(V9fsPDU *pdu, V9fsPath *path,
- V9fsString *xattr_name, void *value,
- size_t size, int flags)
+int coroutine_fn v9fs_co_lsetxattr(V9fsPDU *pdu, V9fsPath *path,
+ V9fsString *xattr_name, void *value,
+ size_t size, int flags)
{
int err;
V9fsState *s = pdu->s;
@@ -85,8 +86,8 @@ int v9fs_co_lsetxattr(V9fsPDU *pdu, V9fsPath *path,
return err;
}
-int v9fs_co_lremovexattr(V9fsPDU *pdu, V9fsPath *path,
- V9fsString *xattr_name)
+int coroutine_fn v9fs_co_lremovexattr(V9fsPDU *pdu, V9fsPath *path,
+ V9fsString *xattr_name)
{
int err;
V9fsState *s = pdu->s;
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 05/12] 9pfs: use coroutine_fn annotation in hw/9pfs/9p.[ch]
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (3 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 04/12] 9pfs: use coroutine_fn annotation in hw/9pfs/co*.[ch] Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 06/12] 9pfs: drop useless check in pdu_free() Greg Kurz
` (7 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz
All these functions either call the v9fs_co_* functions which have the
coroutine_fn annotation, or pdu_complete() which calls qemu_co_queue_next().
Let's mark them to make it obvious they execute in coroutine context.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 117 +++++++++++++++++++++++++++++++----------------------------
hw/9pfs/9p.h | 2 +-
2 files changed, 62 insertions(+), 57 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index ff94a6272c3c..f2ab1dfab2b2 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -236,7 +236,7 @@ static size_t v9fs_string_size(V9fsString *str)
/*
* returns 0 if fid got re-opened, 1 if not, < 0 on error */
-static int v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f)
+static int coroutine_fn v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f)
{
int err = 1;
if (f->fid_type == P9_FID_FILE) {
@@ -255,7 +255,7 @@ static int v9fs_reopen_fid(V9fsPDU *pdu, V9fsFidState *f)
return err;
}
-static V9fsFidState *get_fid(V9fsPDU *pdu, int32_t fid)
+static V9fsFidState *coroutine_fn get_fid(V9fsPDU *pdu, int32_t fid)
{
int err;
V9fsFidState *f;
@@ -321,7 +321,7 @@ static V9fsFidState *alloc_fid(V9fsState *s, int32_t fid)
return f;
}
-static int v9fs_xattr_fid_clunk(V9fsPDU *pdu, V9fsFidState *fidp)
+static int coroutine_fn v9fs_xattr_fid_clunk(V9fsPDU *pdu, V9fsFidState *fidp)
{
int retval = 0;
@@ -353,7 +353,7 @@ free_value:
return retval;
}
-static int free_fid(V9fsPDU *pdu, V9fsFidState *fidp)
+static int coroutine_fn free_fid(V9fsPDU *pdu, V9fsFidState *fidp)
{
int retval = 0;
@@ -374,7 +374,7 @@ static int free_fid(V9fsPDU *pdu, V9fsFidState *fidp)
return retval;
}
-static int put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
+static int coroutine_fn put_fid(V9fsPDU *pdu, V9fsFidState *fidp)
{
BUG_ON(!fidp->ref);
fidp->ref--;
@@ -418,7 +418,7 @@ static V9fsFidState *clunk_fid(V9fsState *s, int32_t fid)
return fidp;
}
-void v9fs_reclaim_fd(V9fsPDU *pdu)
+void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu)
{
int reclaim_count = 0;
V9fsState *s = pdu->s;
@@ -499,7 +499,7 @@ void v9fs_reclaim_fd(V9fsPDU *pdu)
}
}
-static int v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path)
+static int coroutine_fn v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path)
{
int err;
V9fsState *s = pdu->s;
@@ -532,7 +532,7 @@ static int v9fs_mark_fids_unreclaim(V9fsPDU *pdu, V9fsPath *path)
return 0;
}
-static void virtfs_reset(V9fsPDU *pdu)
+static void coroutine_fn virtfs_reset(V9fsPDU *pdu)
{
V9fsState *s = pdu->s;
V9fsFidState *fidp = NULL;
@@ -598,7 +598,8 @@ static void stat_to_qid(const struct stat *stbuf, V9fsQID *qidp)
}
}
-static int fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp, V9fsQID *qidp)
+static int coroutine_fn fid_to_qid(V9fsPDU *pdu, V9fsFidState *fidp,
+ V9fsQID *qidp)
{
struct stat stbuf;
int err;
@@ -643,7 +644,7 @@ void pdu_free(V9fsPDU *pdu)
* because we always expect to have enough space to encode
* error details
*/
-static void pdu_complete(V9fsPDU *pdu, ssize_t len)
+static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len)
{
int8_t id = pdu->id + 1; /* Response */
V9fsState *s = pdu->s;
@@ -810,9 +811,9 @@ static uint32_t stat_to_v9mode(const struct stat *stbuf)
return mode;
}
-static int stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
- const struct stat *stbuf,
- V9fsStat *v9stat)
+static int coroutine_fn stat_to_v9stat(V9fsPDU *pdu, V9fsPath *name,
+ const struct stat *stbuf,
+ V9fsStat *v9stat)
{
int err;
const char *str;
@@ -941,7 +942,7 @@ static inline bool is_ro_export(FsContext *ctx)
return ctx->export_flags & V9FS_RDONLY;
}
-static void v9fs_version(void *opaque)
+static void coroutine_fn v9fs_version(void *opaque)
{
ssize_t err;
V9fsPDU *pdu = opaque;
@@ -979,7 +980,7 @@ out:
v9fs_string_free(&version);
}
-static void v9fs_attach(void *opaque)
+static void coroutine_fn v9fs_attach(void *opaque)
{
V9fsPDU *pdu = opaque;
V9fsState *s = pdu->s;
@@ -1045,7 +1046,7 @@ out_nofid:
v9fs_string_free(&aname);
}
-static void v9fs_stat(void *opaque)
+static void coroutine_fn v9fs_stat(void *opaque)
{
int32_t fid;
V9fsStat v9stat;
@@ -1089,7 +1090,7 @@ out_nofid:
pdu_complete(pdu, err);
}
-static void v9fs_getattr(void *opaque)
+static void coroutine_fn v9fs_getattr(void *opaque)
{
int32_t fid;
size_t offset = 7;
@@ -1165,7 +1166,7 @@ out_nofid:
#define P9_ATTR_MASK 127
-static void v9fs_setattr(void *opaque)
+static void coroutine_fn v9fs_setattr(void *opaque)
{
int err = 0;
int32_t fid;
@@ -1283,7 +1284,7 @@ static bool not_same_qid(const V9fsQID *qid1, const V9fsQID *qid2)
qid1->path != qid2->path;
}
-static void v9fs_walk(void *opaque)
+static void coroutine_fn v9fs_walk(void *opaque)
{
int name_idx;
V9fsQID *qids = NULL;
@@ -1397,7 +1398,7 @@ out_nofid:
}
}
-static int32_t get_iounit(V9fsPDU *pdu, V9fsPath *path)
+static int32_t coroutine_fn get_iounit(V9fsPDU *pdu, V9fsPath *path)
{
struct statfs stbuf;
int32_t iounit = 0;
@@ -1417,7 +1418,7 @@ static int32_t get_iounit(V9fsPDU *pdu, V9fsPath *path)
return iounit;
}
-static void v9fs_open(void *opaque)
+static void coroutine_fn v9fs_open(void *opaque)
{
int flags;
int32_t fid;
@@ -1507,7 +1508,7 @@ out_nofid:
pdu_complete(pdu, err);
}
-static void v9fs_lcreate(void *opaque)
+static void coroutine_fn v9fs_lcreate(void *opaque)
{
int32_t dfid, flags, mode;
gid_t gid;
@@ -1604,7 +1605,7 @@ out_nofid:
pdu_complete(pdu, err);
}
-static void v9fs_clunk(void *opaque)
+static void coroutine_fn v9fs_clunk(void *opaque)
{
int err;
int32_t fid;
@@ -1673,8 +1674,9 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
return offset;
}
-static int v9fs_do_readdir_with_stat(V9fsPDU *pdu,
- V9fsFidState *fidp, uint32_t max_count)
+static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU *pdu,
+ V9fsFidState *fidp,
+ uint32_t max_count)
{
V9fsPath path;
V9fsStat v9stat;
@@ -1764,7 +1766,7 @@ static void v9fs_init_qiov_from_pdu(QEMUIOVector *qiov, V9fsPDU *pdu,
qemu_iovec_concat(qiov, &elem, skip, size);
}
-static void v9fs_read(void *opaque)
+static void coroutine_fn v9fs_read(void *opaque)
{
int32_t fid;
uint64_t off;
@@ -1858,8 +1860,8 @@ static size_t v9fs_readdir_data_size(V9fsString *name)
return 24 + v9fs_string_size(name);
}
-static int v9fs_do_readdir(V9fsPDU *pdu,
- V9fsFidState *fidp, int32_t max_count)
+static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, V9fsFidState *fidp,
+ int32_t max_count)
{
size_t size;
V9fsQID qid;
@@ -1928,7 +1930,7 @@ static int v9fs_do_readdir(V9fsPDU *pdu,
return count;
}
-static void v9fs_readdir(void *opaque)
+static void coroutine_fn v9fs_readdir(void *opaque)
{
int32_t fid;
V9fsFidState *fidp;
@@ -2024,7 +2026,7 @@ out:
return err;
}
-static void v9fs_write(void *opaque)
+static void coroutine_fn v9fs_write(void *opaque)
{
ssize_t err;
int32_t fid;
@@ -2107,7 +2109,7 @@ out_nofid:
pdu_complete(pdu, err);
}
-static void v9fs_create(void *opaque)
+static void coroutine_fn v9fs_create(void *opaque)
{
int32_t fid;
int err = 0;
@@ -2287,7 +2289,7 @@ out_nofid:
v9fs_path_free(&path);
}
-static void v9fs_symlink(void *opaque)
+static void coroutine_fn v9fs_symlink(void *opaque)
{
V9fsPDU *pdu = opaque;
V9fsString name;
@@ -2376,7 +2378,7 @@ static void v9fs_flush(void *opaque)
pdu_complete(pdu, 7);
}
-static void v9fs_link(void *opaque)
+static void coroutine_fn v9fs_link(void *opaque)
{
V9fsPDU *pdu = opaque;
int32_t dfid, oldfid;
@@ -2425,7 +2427,7 @@ out_nofid:
}
/* Only works with path name based fid */
-static void v9fs_remove(void *opaque)
+static void coroutine_fn v9fs_remove(void *opaque)
{
int32_t fid;
int err = 0;
@@ -2469,7 +2471,7 @@ out_nofid:
pdu_complete(pdu, err);
}
-static void v9fs_unlinkat(void *opaque)
+static void coroutine_fn v9fs_unlinkat(void *opaque)
{
int err = 0;
V9fsString name;
@@ -2532,8 +2534,9 @@ out_nofid:
/* Only works with path name based fid */
-static int v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
- int32_t newdirfid, V9fsString *name)
+static int coroutine_fn v9fs_complete_rename(V9fsPDU *pdu, V9fsFidState *fidp,
+ int32_t newdirfid,
+ V9fsString *name)
{
char *end;
int err = 0;
@@ -2590,7 +2593,7 @@ out_nofid:
}
/* Only works with path name based fid */
-static void v9fs_rename(void *opaque)
+static void coroutine_fn v9fs_rename(void *opaque)
{
int32_t fid;
ssize_t err = 0;
@@ -2641,9 +2644,10 @@ out_nofid:
v9fs_string_free(&name);
}
-static void v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
- V9fsString *old_name, V9fsPath *newdir,
- V9fsString *new_name)
+static void coroutine_fn v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
+ V9fsString *old_name,
+ V9fsPath *newdir,
+ V9fsString *new_name)
{
V9fsFidState *tfidp;
V9fsPath oldpath, newpath;
@@ -2669,9 +2673,10 @@ static void v9fs_fix_fid_paths(V9fsPDU *pdu, V9fsPath *olddir,
v9fs_path_free(&newpath);
}
-static int v9fs_complete_renameat(V9fsPDU *pdu, int32_t olddirfid,
- V9fsString *old_name, int32_t newdirfid,
- V9fsString *new_name)
+static int coroutine_fn v9fs_complete_renameat(V9fsPDU *pdu, int32_t olddirfid,
+ V9fsString *old_name,
+ int32_t newdirfid,
+ V9fsString *new_name)
{
int err = 0;
V9fsState *s = pdu->s;
@@ -2712,7 +2717,7 @@ out:
return err;
}
-static void v9fs_renameat(void *opaque)
+static void coroutine_fn v9fs_renameat(void *opaque)
{
ssize_t err = 0;
size_t offset = 7;
@@ -2754,7 +2759,7 @@ out_err:
v9fs_string_free(&new_name);
}
-static void v9fs_wstat(void *opaque)
+static void coroutine_fn v9fs_wstat(void *opaque)
{
int32_t fid;
int err = 0;
@@ -2893,7 +2898,7 @@ static int v9fs_fill_statfs(V9fsState *s, V9fsPDU *pdu, struct statfs *stbuf)
fsid_val, f_namelen);
}
-static void v9fs_statfs(void *opaque)
+static void coroutine_fn v9fs_statfs(void *opaque)
{
int32_t fid;
ssize_t retval = 0;
@@ -2927,7 +2932,7 @@ out_nofid:
pdu_complete(pdu, retval);
}
-static void v9fs_mknod(void *opaque)
+static void coroutine_fn v9fs_mknod(void *opaque)
{
int mode;
@@ -2993,7 +2998,7 @@ out_nofid:
* do any thing in * qemu 9p server side lock code path.
* So when a TLOCK request comes, always return success
*/
-static void v9fs_lock(void *opaque)
+static void coroutine_fn v9fs_lock(void *opaque)
{
int8_t status;
V9fsFlock flock;
@@ -3046,7 +3051,7 @@ out_nofid:
* When a TGETLOCK request comes, always return success because all lock
* handling is done by client's VFS layer.
*/
-static void v9fs_getlock(void *opaque)
+static void coroutine_fn v9fs_getlock(void *opaque)
{
size_t offset = 7;
struct stat stbuf;
@@ -3091,7 +3096,7 @@ out_nofid:
v9fs_string_free(&glock.client_id);
}
-static void v9fs_mkdir(void *opaque)
+static void coroutine_fn v9fs_mkdir(void *opaque)
{
V9fsPDU *pdu = opaque;
size_t offset = 7;
@@ -3145,7 +3150,7 @@ out_nofid:
v9fs_string_free(&name);
}
-static void v9fs_xattrwalk(void *opaque)
+static void coroutine_fn v9fs_xattrwalk(void *opaque)
{
int64_t size;
V9fsString name;
@@ -3251,7 +3256,7 @@ out_nofid:
v9fs_string_free(&name);
}
-static void v9fs_xattrcreate(void *opaque)
+static void coroutine_fn v9fs_xattrcreate(void *opaque)
{
int flags;
int32_t fid;
@@ -3291,7 +3296,7 @@ out_nofid:
v9fs_string_free(&name);
}
-static void v9fs_readlink(void *opaque)
+static void coroutine_fn v9fs_readlink(void *opaque)
{
V9fsPDU *pdu = opaque;
size_t offset = 7;
@@ -3367,13 +3372,13 @@ static CoroutineEntry *pdu_co_handlers[] = {
[P9_TREMOVE] = v9fs_remove,
};
-static void v9fs_op_not_supp(void *opaque)
+static void coroutine_fn v9fs_op_not_supp(void *opaque)
{
V9fsPDU *pdu = opaque;
pdu_complete(pdu, -EOPNOTSUPP);
}
-static void v9fs_fs_ro(void *opaque)
+static void coroutine_fn v9fs_fs_ro(void *opaque)
{
V9fsPDU *pdu = opaque;
pdu_complete(pdu, -EROFS);
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index 5225b4f12071..c4df66d1c4ea 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -324,7 +324,7 @@ static inline uint8_t v9fs_request_cancelled(V9fsPDU *pdu)
return pdu->cancelled;
}
-void v9fs_reclaim_fd(V9fsPDU *pdu);
+void coroutine_fn v9fs_reclaim_fd(V9fsPDU *pdu);
void v9fs_path_init(V9fsPath *path);
void v9fs_path_free(V9fsPath *path);
void v9fs_path_sprintf(V9fsPath *path, const char *fmt, ...);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 06/12] 9pfs: drop useless check in pdu_free()
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (4 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 05/12] 9pfs: use coroutine_fn annotation in hw/9pfs/9p.[ch] Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 07/12] 9pfs: only free completed request if not flushed Greg Kurz
` (6 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz
Out of the three users of pdu_free(), none ever passes a NULL pointer to
this function.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index f2ab1dfab2b2..df8aa726c9a1 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -626,16 +626,14 @@ V9fsPDU *pdu_alloc(V9fsState *s)
void pdu_free(V9fsPDU *pdu)
{
- if (pdu) {
- V9fsState *s = pdu->s;
- /*
- * Cancelled pdu are added back to the freelist
- * by flush request .
- */
- if (!pdu->cancelled) {
- QLIST_REMOVE(pdu, next);
- QLIST_INSERT_HEAD(&s->free_list, pdu, next);
- }
+ V9fsState *s = pdu->s;
+ /*
+ * Cancelled pdu are added back to the freelist
+ * by flush request .
+ */
+ if (!pdu->cancelled) {
+ QLIST_REMOVE(pdu, next);
+ QLIST_INSERT_HEAD(&s->free_list, pdu, next);
}
}
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 07/12] 9pfs: only free completed request if not flushed
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (5 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 06/12] 9pfs: drop useless check in pdu_free() Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 08/12] virtio-9p: add reset handler Greg Kurz
` (5 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz
If a PDU has a flush request pending, the current code calls pdu_free()
twice:
1) pdu_complete()->pdu_free() with pdu->cancelled set, which does nothing
2) v9fs_flush()->pdu_free() with pdu->cancelled cleared, which moves the
PDU back to the free list.
This works but it complexifies the logic of pdu_free().
With this patch, pdu_complete() only calls pdu_free() if no flush request
is pending, i.e. qemu_co_queue_next() returns false.
Since pdu_free() is now supposed to be called with pdu->cancelled cleared,
the check in pdu_free() is dropped and replaced by an assertion.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 18 +++++++-----------
1 file changed, 7 insertions(+), 11 deletions(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index df8aa726c9a1..f0dc2ce589cd 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -627,14 +627,10 @@ V9fsPDU *pdu_alloc(V9fsState *s)
void pdu_free(V9fsPDU *pdu)
{
V9fsState *s = pdu->s;
- /*
- * Cancelled pdu are added back to the freelist
- * by flush request .
- */
- if (!pdu->cancelled) {
- QLIST_REMOVE(pdu, next);
- QLIST_INSERT_HEAD(&s->free_list, pdu, next);
- }
+
+ g_assert(!pdu->cancelled);
+ QLIST_REMOVE(pdu, next);
+ QLIST_INSERT_HEAD(&s->free_list, pdu, next);
}
/*
@@ -679,9 +675,9 @@ static void coroutine_fn pdu_complete(V9fsPDU *pdu, ssize_t len)
pdu_push_and_notify(pdu);
/* Now wakeup anybody waiting in flush for this request */
- qemu_co_queue_next(&pdu->complete);
-
- pdu_free(pdu);
+ if (!qemu_co_queue_next(&pdu->complete)) {
+ pdu_free(pdu);
+ }
}
static mode_t v9mode_to_mode(uint32_t mode, V9fsString *extension)
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 08/12] virtio-9p: add reset handler
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (6 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 07/12] 9pfs: only free completed request if not flushed Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 09/12] 9pfs: fix information leak in xattr read Greg Kurz
` (4 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz
Virtio devices should implement the VirtIODevice->reset() function to
perform necessary cleanup actions and to bring the device to a quiescent
state.
In the case of the virtio-9p device, this means:
- emptying the list of active PDUs (i.e. draining all in-flight I/O)
- freeing all fids (i.e. close open file descriptors and free memory)
That's what this patch does.
The reset handler first waits for all active PDUs to complete. Since
completion happens in the QEMU global aio context, we just have to
loop around aio_poll() until the active list is empty.
The freeing part involves some actions to be performed on the backend,
like closing file descriptors or flushing extended attributes to the
underlying filesystem. The virtfs_reset() function already does the
job: it calls free_fid() for all open fids not involved in an ongoing
I/O operation. We are sure this is the case since we have drained
the PDU active list.
The current code implements all backend accesses with coroutines, but we
want to stay synchronous on the reset path. We can either change the
current code to be able to run when not in coroutine context, or create
a coroutine context and wait for virtfs_reset() to complete. This patch
goes for the latter because it results in simpler code.
Note that we also need to create a dummy PDU because it is also an API
to pass the FsContext pointer to all backend callbacks.
Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
hw/9pfs/9p.c | 30 ++++++++++++++++++++++++++++++
hw/9pfs/9p.h | 1 +
hw/9pfs/virtio-9p-device.c | 8 ++++++++
3 files changed, 39 insertions(+)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index f0dc2ce589cd..26aa7d564815 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3522,6 +3522,36 @@ void v9fs_device_unrealize_common(V9fsState *s, Error **errp)
g_free(s->tag);
}
+typedef struct VirtfsCoResetData {
+ V9fsPDU pdu;
+ bool done;
+} VirtfsCoResetData;
+
+static void coroutine_fn virtfs_co_reset(void *opaque)
+{
+ VirtfsCoResetData *data = opaque;
+
+ virtfs_reset(&data->pdu);
+ data->done = true;
+}
+
+void v9fs_reset(V9fsState *s)
+{
+ VirtfsCoResetData data = { .pdu = { .s = s }, .done = false };
+ Coroutine *co;
+
+ while (!QLIST_EMPTY(&s->active_list)) {
+ aio_poll(qemu_get_aio_context(), true);
+ }
+
+ co = qemu_coroutine_create(virtfs_co_reset, &data);
+ qemu_coroutine_enter(co);
+
+ while (!data.done) {
+ aio_poll(qemu_get_aio_context(), true);
+ }
+}
+
static void __attribute__((__constructor__)) v9fs_set_fd_limit(void)
{
struct rlimit rlim;
diff --git a/hw/9pfs/9p.h b/hw/9pfs/9p.h
index c4df66d1c4ea..2523a445f81f 100644
--- a/hw/9pfs/9p.h
+++ b/hw/9pfs/9p.h
@@ -339,5 +339,6 @@ ssize_t pdu_unmarshal(V9fsPDU *pdu, size_t offset, const char *fmt, ...);
V9fsPDU *pdu_alloc(V9fsState *s);
void pdu_free(V9fsPDU *pdu);
void pdu_submit(V9fsPDU *pdu);
+void v9fs_reset(V9fsState *s);
#endif
diff --git a/hw/9pfs/virtio-9p-device.c b/hw/9pfs/virtio-9p-device.c
index e98dd0c4c0af..1782e4a2277f 100644
--- a/hw/9pfs/virtio-9p-device.c
+++ b/hw/9pfs/virtio-9p-device.c
@@ -141,6 +141,13 @@ static void virtio_9p_device_unrealize(DeviceState *dev, Error **errp)
v9fs_device_unrealize_common(s, errp);
}
+static void virtio_9p_reset(VirtIODevice *vdev)
+{
+ V9fsVirtioState *v = (V9fsVirtioState *)vdev;
+
+ v9fs_reset(&v->state);
+}
+
ssize_t virtio_pdu_vmarshal(V9fsPDU *pdu, size_t offset,
const char *fmt, va_list ap)
{
@@ -207,6 +214,7 @@ static void virtio_9p_class_init(ObjectClass *klass, void *data)
vdc->unrealize = virtio_9p_device_unrealize;
vdc->get_features = virtio_9p_get_features;
vdc->get_config = virtio_9p_get_config;
+ vdc->reset = virtio_9p_reset;
}
static const TypeInfo virtio_device_info = {
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 09/12] 9pfs: fix information leak in xattr read
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (7 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 08/12] virtio-9p: add reset handler Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 10/12] 9pfs: fix memory leak in v9fs_xattrcreate Greg Kurz
` (3 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz, Li Qiang
From: Li Qiang <liqiang6-s@360.cn>
9pfs uses g_malloc() to allocate the xattr memory space, if the guest
reads this memory before writing to it, this will leak host heap memory
to the guest. This patch avoid this.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Greg Kurz <groug@kaod.org>
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 26aa7d564815..bf23b011a8eb 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3282,7 +3282,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
xattr_fidp->fs.xattr.flags = flags;
v9fs_string_init(&xattr_fidp->fs.xattr.name);
v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name);
- xattr_fidp->fs.xattr.value = g_malloc(size);
+ xattr_fidp->fs.xattr.value = g_malloc0(size);
err = offset;
put_fid(pdu, file_fidp);
out_nofid:
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 10/12] 9pfs: fix memory leak in v9fs_xattrcreate
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (8 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 09/12] 9pfs: fix information leak in xattr read Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 11/12] 9pfs: fix memory leak in v9fs_link Greg Kurz
` (2 subsequent siblings)
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz, Li Qiang
From: Li Qiang <liqiang6-s@360.cn>
The 'fs.xattr.value' field in V9fsFidState object doesn't consider the
situation that this field has been allocated previously. Every time, it
will be allocated directly. This leads to a host memory leak issue if
the client sends another Txattrcreate message with the same fid number
before the fid from the previous time got clunked.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Greg Kurz <groug@kaod.org>
[groug, updated the changelog to indicate how the leak can occur]
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index bf23b011a8eb..66135cf12183 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -3282,6 +3282,7 @@ static void coroutine_fn v9fs_xattrcreate(void *opaque)
xattr_fidp->fs.xattr.flags = flags;
v9fs_string_init(&xattr_fidp->fs.xattr.name);
v9fs_string_copy(&xattr_fidp->fs.xattr.name, &name);
+ g_free(xattr_fidp->fs.xattr.value);
xattr_fidp->fs.xattr.value = g_malloc0(size);
err = offset;
put_fid(pdu, file_fidp);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 11/12] 9pfs: fix memory leak in v9fs_link
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (9 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 10/12] 9pfs: fix memory leak in v9fs_xattrcreate Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 15:05 ` [Qemu-devel] [PULL 12/12] 9pfs: fix memory leak in v9fs_write Greg Kurz
2016-10-17 16:17 ` [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Peter Maydell
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz, Li Qiang
From: Li Qiang <liqiang6-s@360.cn>
The v9fs_link() function keeps a reference on the source fid object. This
causes a memory leak since the reference never goes down to 0. This patch
fixes the issue.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Greg Kurz <groug@kaod.org>
[groug, rephrased the changelog]
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 66135cf12183..d43a55223492 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -2413,6 +2413,7 @@ static void coroutine_fn v9fs_link(void *opaque)
if (!err) {
err = offset;
}
+ put_fid(pdu, oldfidp);
out:
put_fid(pdu, dfidp);
out_nofid:
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* [Qemu-devel] [PULL 12/12] 9pfs: fix memory leak in v9fs_write
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (10 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 11/12] 9pfs: fix memory leak in v9fs_link Greg Kurz
@ 2016-10-17 15:05 ` Greg Kurz
2016-10-17 16:17 ` [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Peter Maydell
12 siblings, 0 replies; 14+ messages in thread
From: Greg Kurz @ 2016-10-17 15:05 UTC (permalink / raw)
To: qemu-devel; +Cc: Peter Maydell, Aneesh Kumar K.V, Greg Kurz, Li Qiang
From: Li Qiang <liqiang6-s@360.cn>
If an error occurs when marshalling the transfer length to the guest, the
v9fs_write() function doesn't free an IO vector, thus leading to a memory
leak. This patch fixes the issue.
Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Greg Kurz <groug@kaod.org>
[groug, rephrased the changelog]
Signed-off-by: Greg Kurz <groug@kaod.org>
---
hw/9pfs/9p.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index d43a55223492..e88cf257a2b9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -2090,7 +2090,7 @@ static void coroutine_fn v9fs_write(void *opaque)
offset = 7;
err = pdu_marshal(pdu, offset, "d", total);
if (err < 0) {
- goto out;
+ goto out_qiov;
}
err += offset;
trace_v9fs_write_return(pdu->tag, pdu->id, total, err);
--
2.5.5
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017
2016-10-17 15:05 [Qemu-devel] [PULL 00/12] 9p patches for 2.8 20161017 Greg Kurz
` (11 preceding siblings ...)
2016-10-17 15:05 ` [Qemu-devel] [PULL 12/12] 9pfs: fix memory leak in v9fs_write Greg Kurz
@ 2016-10-17 16:17 ` Peter Maydell
12 siblings, 0 replies; 14+ messages in thread
From: Peter Maydell @ 2016-10-17 16:17 UTC (permalink / raw)
To: Greg Kurz; +Cc: QEMU Developers, Aneesh Kumar K.V
On 17 October 2016 at 16:05, Greg Kurz <groug@kaod.org> wrote:
> The following changes since commit ad728364e3916e1159ee94e5cd82b7a9c81d2dcc:
>
> Merge remote-tracking branch 'remotes/famz/tags/for-upstream' into staging (2016-10-17 11:56:18 +0100)
>
> are available in the git repository at:
>
> https://github.com/gkurz/qemu.git tags/for-upstream
>
> for you to fetch changes up to fdfcc9aeea1492f4b819a24c94dfb678145b1bf9:
>
> 9pfs: fix memory leak in v9fs_write (2016-10-17 14:13:58 +0200)
>
> ----------------------------------------------------------------
> This pull request contains:
> - a patch to add a vdc->reset() handler to virtio-9p
> - a bunch of patches to fix various memory leaks (thanks to Li Qiang)
> - some code cleanups for 9pfs
>
> ----------------------------------------------------------------
> Greg Kurz (6):
> 9pfs: fsdev: drop useless extern annotation for functions
> 9pfs: use coroutine_fn annotation in hw/9pfs/co*.[ch]
> 9pfs: use coroutine_fn annotation in hw/9pfs/9p.[ch]
> 9pfs: drop useless check in pdu_free()
> 9pfs: only free completed request if not flushed
> virtio-9p: add reset handler
>
> Li Qiang (6):
> 9pfs: allocate space for guest originated empty strings
> 9pfs: fix potential host memory leak in v9fs_read
> 9pfs: fix information leak in xattr read
> 9pfs: fix memory leak in v9fs_xattrcreate
> 9pfs: fix memory leak in v9fs_link
> 9pfs: fix memory leak in v9fs_write
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 14+ messages in thread