* [PATCH net 1/2] vhost: check owner before we overwrite ubuf_info
2013-06-06 12:20 [PATCH net 0/2] vhost fixes for 3.10 Michael S. Tsirkin
@ 2013-06-06 12:20 ` Michael S. Tsirkin
2013-06-11 9:46 ` David Miller
2013-06-06 12:20 ` [PATCH net 2/2] vhost: fix ubuf_info cleanup Michael S. Tsirkin
2013-06-06 18:50 ` [PATCH net 0/2] vhost fixes for 3.10 Tommi Rantala
2 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-06-06 12:20 UTC (permalink / raw)
To: netdev; +Cc: Tommi Rantala, kvm, linux-kernel, virtualization, David Miller
If device has an owner, we shouldn't touch ubuf_info
since it might be in use.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 4 ++++
drivers/vhost/vhost.c | 8 +++++++-
drivers/vhost/vhost.h | 1 +
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 2b51e23..6b00f64 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1053,6 +1053,10 @@ static long vhost_net_set_owner(struct vhost_net *n)
int r;
mutex_lock(&n->dev.mutex);
+ if (vhost_dev_has_owner(&n->dev)) {
+ r = -EBUSY;
+ goto out;
+ }
r = vhost_net_set_ubuf_info(n);
if (r)
goto out;
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index beee7f5..60aa5ad 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -344,13 +344,19 @@ static int vhost_attach_cgroups(struct vhost_dev *dev)
}
/* Caller should have device mutex */
+bool vhost_dev_has_owner(struct vhost_dev *dev)
+{
+ return dev->mm;
+}
+
+/* Caller should have device mutex */
long vhost_dev_set_owner(struct vhost_dev *dev)
{
struct task_struct *worker;
int err;
/* Is there an owner already? */
- if (dev->mm) {
+ if (vhost_dev_has_owner(dev)) {
err = -EBUSY;
goto err_mm;
}
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index a7ad635..64adcf9 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -133,6 +133,7 @@ struct vhost_dev {
long vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs, int nvqs);
long vhost_dev_set_owner(struct vhost_dev *dev);
+bool vhost_dev_has_owner(struct vhost_dev *dev);
long vhost_dev_check_owner(struct vhost_dev *);
struct vhost_memory *vhost_dev_reset_owner_prepare(void);
void vhost_dev_reset_owner(struct vhost_dev *, struct vhost_memory *);
--
MST
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net 2/2] vhost: fix ubuf_info cleanup
2013-06-06 12:20 [PATCH net 0/2] vhost fixes for 3.10 Michael S. Tsirkin
2013-06-06 12:20 ` [PATCH net 1/2] vhost: check owner before we overwrite ubuf_info Michael S. Tsirkin
@ 2013-06-06 12:20 ` Michael S. Tsirkin
2013-06-11 9:46 ` David Miller
2013-06-06 18:50 ` [PATCH net 0/2] vhost fixes for 3.10 Tommi Rantala
2 siblings, 1 reply; 6+ messages in thread
From: Michael S. Tsirkin @ 2013-06-06 12:20 UTC (permalink / raw)
To: netdev; +Cc: Tommi Rantala, kvm, linux-kernel, virtualization, David Miller
vhost_net_clear_ubuf_info didn't clear ubuf_info
after kfree, this could trigger double free.
Fix this and simplify this code to make it more robust: make sure
ubuf info is always freed through vhost_net_clear_ubuf_info.
Reported-by: Tommi Rantala <tt.rantala@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
drivers/vhost/net.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 6b00f64..7fc47f7 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -155,14 +155,11 @@ static void vhost_net_ubuf_put_and_wait(struct vhost_net_ubuf_ref *ubufs)
static void vhost_net_clear_ubuf_info(struct vhost_net *n)
{
-
- bool zcopy;
int i;
- for (i = 0; i < n->dev.nvqs; ++i) {
- zcopy = vhost_net_zcopy_mask & (0x1 << i);
- if (zcopy)
- kfree(n->vqs[i].ubuf_info);
+ for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
+ kfree(n->vqs[i].ubuf_info);
+ n->vqs[i].ubuf_info = NULL;
}
}
@@ -171,7 +168,7 @@ int vhost_net_set_ubuf_info(struct vhost_net *n)
bool zcopy;
int i;
- for (i = 0; i < n->dev.nvqs; ++i) {
+ for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
zcopy = vhost_net_zcopy_mask & (0x1 << i);
if (!zcopy)
continue;
@@ -183,12 +180,7 @@ int vhost_net_set_ubuf_info(struct vhost_net *n)
return 0;
err:
- while (i--) {
- zcopy = vhost_net_zcopy_mask & (0x1 << i);
- if (!zcopy)
- continue;
- kfree(n->vqs[i].ubuf_info);
- }
+ vhost_net_clear_ubuf_info(n);
return -ENOMEM;
}
@@ -196,12 +188,12 @@ void vhost_net_vq_reset(struct vhost_net *n)
{
int i;
+ vhost_net_clear_ubuf_info(n);
+
for (i = 0; i < VHOST_NET_VQ_MAX; i++) {
n->vqs[i].done_idx = 0;
n->vqs[i].upend_idx = 0;
n->vqs[i].ubufs = NULL;
- kfree(n->vqs[i].ubuf_info);
- n->vqs[i].ubuf_info = NULL;
n->vqs[i].vhost_hlen = 0;
n->vqs[i].sock_hlen = 0;
}
--
MST
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH net 0/2] vhost fixes for 3.10
2013-06-06 12:20 [PATCH net 0/2] vhost fixes for 3.10 Michael S. Tsirkin
2013-06-06 12:20 ` [PATCH net 1/2] vhost: check owner before we overwrite ubuf_info Michael S. Tsirkin
2013-06-06 12:20 ` [PATCH net 2/2] vhost: fix ubuf_info cleanup Michael S. Tsirkin
@ 2013-06-06 18:50 ` Tommi Rantala
2 siblings, 0 replies; 6+ messages in thread
From: Tommi Rantala @ 2013-06-06 18:50 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: kvm, netdev, LKML, virtualization, David Miller
2013/6/6 Michael S. Tsirkin <mst@redhat.com>:
> Two patches fixing the fallout from the vhost cleanup in 3.10.
> Thanks to Tommi Rantala who reported the issue.
>
> Tommi, could you please confirm this fixes the crashes for you?
Confirmed! With the two patches applied, I can no longer reproduce the
crash with trinity.
Thanks!
Tommi
> Michael S. Tsirkin (2):
> vhost: check owner before we overwrite ubuf_info
> vhost: fix ubuf_info cleanup
>
> drivers/vhost/net.c | 26 +++++++++++---------------
> drivers/vhost/vhost.c | 8 +++++++-
> drivers/vhost/vhost.h | 1 +
> 3 files changed, 19 insertions(+), 16 deletions(-)
>
> --
> MST
>
^ permalink raw reply [flat|nested] 6+ messages in thread