* [PATCH net 0/2] vhost fixes for 3.10
@ 2013-06-06 12:20 Michael S. Tsirkin
2013-06-06 12:20 ` [PATCH net 1/2] vhost: check owner before we overwrite ubuf_info Michael S. Tsirkin
` (2 more replies)
0 siblings, 3 replies; 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
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?
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
* [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
* Re: [PATCH net 1/2] vhost: check owner before we overwrite ubuf_info
2013-06-06 12:20 ` [PATCH net 1/2] vhost: check owner before we overwrite ubuf_info Michael S. Tsirkin
@ 2013-06-11 9:46 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-06-11 9:46 UTC (permalink / raw)
To: mst; +Cc: tt.rantala, kvm, netdev, linux-kernel, virtualization
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 6 Jun 2013 15:20:39 +0300
> 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>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] vhost: fix ubuf_info cleanup
2013-06-06 12:20 ` [PATCH net 2/2] vhost: fix ubuf_info cleanup Michael S. Tsirkin
@ 2013-06-11 9:46 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2013-06-11 9:46 UTC (permalink / raw)
To: mst; +Cc: tt.rantala, kvm, netdev, linux-kernel, virtualization
From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Thu, 6 Jun 2013 15:20:46 +0300
> 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>
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-11 9:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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-11 9:46 ` David Miller
2013-06-06 12:20 ` [PATCH net 2/2] vhost: fix ubuf_info cleanup 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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).