vhost: fix error handling in vring ioctls

PATCH In review Michael S. Tsirkin 2 participants 3 messages Mar 17, 2010

Subsystems (5)

Networking Network drivers KVM virtio core +1 more subsystems

lore

Share & tools

lore

Michael S. Tsirkin

author maintainer

2010-03-17 14:42 UTC #0 lore raw

PATCH 1/1 vhost: fix error handling in vring ioctls

Stanse found a locking problem in vhost_set_vring:
several returns from VHOST_SET_VRING_KICK, VHOST_SET_VRING_CALL, VHOST_SET_VRING_ERR with the vq->mutex held.
Fix these up.

Reported-by Jiri Slaby <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
 drivers/vhost/vhost.c |   18 ++++++++++++------
 1 files changed, 12 insertions(+), 6 deletions(-)

drivers/vhost/vhost.c +12 -6

index 7cd55e0..7bd7a1e 100644
@@ -476,8 +476,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
 		if (r < 0)
 			break;
 		eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-		if (IS_ERR(eventfp))
-			return PTR_ERR(eventfp);
+		if (IS_ERR(eventfp)) {
+			r = PTR_ERR(eventfp);
+			break;
+		}
 		if (eventfp != vq->kick) {
 			pollstop = filep = vq->kick;
 			pollstart = vq->kick = eventfp;

@@ -489,8 +491,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
 		if (r < 0)
 			break;
 		eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-		if (IS_ERR(eventfp))
-			return PTR_ERR(eventfp);
+		if (IS_ERR(eventfp)) {
+			r = PTR_ERR(eventfp);
+			break;
+		}
 		if (eventfp != vq->call) {
 			filep = vq->call;
 			ctx = vq->call_ctx;

@@ -505,8 +509,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
 		if (r < 0)
 			break;
 		eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-		if (IS_ERR(eventfp))
-			return PTR_ERR(eventfp);
+		if (IS_ERR(eventfp)) {
+			r = PTR_ERR(eventfp);
+			break;
+		}
 		if (eventfp != vq->error) {
 			filep = vq->error;
 			vq->error = eventfp;

Signature · 1 line

1.7.0.18.g0d53a5

Laurent Chavey

Acked-by on v1 1/1

2010-03-17 17:54 UTC #1 lore raw

Acked-by [email protected]

Quoted 61 lines · Michael S. Tsirkin

Stanse found a locking problem in vhost_set_vring:
several returns from VHOST_SET_VRING_KICK, VHOST_SET_VRING_CALL, VHOST_SET_VRING_ERR with the vq->mutex held.
Fix these up.

Reported-by Jiri Slaby <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
drivers/vhost/vhost.c | 18 ++++++++++++------

 1 files changed, 12 insertions(+), 6 deletions(-)

drivers/vhost/vhost.c +12 -7

index 7cd55e0..7bd7a1e 100644
@@ -476,8 +476,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
                if (r < 0)
                        break;
                eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-               if (IS_ERR(eventfp))
-                       return PTR_ERR(eventfp);
+               if (IS_ERR(eventfp)) {
+                       r = PTR_ERR(eventfp);
+                       break;
+               }
                if (eventfp != vq->kick) {
                        pollstop = filep = vq->kick;
                        pollstart = vq->kick = eventfp;

@@ -489,8 +491,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
                if (r < 0)
                        break;
                eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-               if (IS_ERR(eventfp))
-                       return PTR_ERR(eventfp);
+               if (IS_ERR(eventfp)) {
+                       r = PTR_ERR(eventfp);
+                       break;
+               }
                if (eventfp != vq->call) {
                        filep = vq->call;
                        ctx = vq->call_ctx;

@@ -505,8 +509,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
                if (r < 0)
                        break;
                eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-               if (IS_ERR(eventfp))
-                       return PTR_ERR(eventfp);
+               if (IS_ERR(eventfp)) {
+                       r = PTR_ERR(eventfp);
+                       break;
+               }
                if (eventfp != vq->error) {
                        filep = vq->error;
                        vq->error = eventfp;
--

1.7.0.18.g0d53a5
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Laurent Chavey

Acked-by

2010-03-17 18:40 UTC #2 lore raw

Acked-by Laurent Chavey <[email protected]>

Quoted 66 lines · Laurent Chavey

Acked-by [email protected]

Quoted 61 lines · Michael S. Tsirkin

Stanse found a locking problem in vhost_set_vring:
several returns from VHOST_SET_VRING_KICK, VHOST_SET_VRING_CALL, VHOST_SET_VRING_ERR with the vq->mutex held.
Fix these up.

Reported-by Jiri Slaby <[email protected]>

Signed-off-by Michael S. Tsirkin <[email protected]>

---
drivers/vhost/vhost.c | 18 ++++++++++++------

 1 files changed, 12 insertions(+), 6 deletions(-)

drivers/vhost/vhost.c +12 -7

index 7cd55e0..7bd7a1e 100644
@@ -476,8 +476,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
                if (r < 0)
                        break;
                eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-               if (IS_ERR(eventfp))
-                       return PTR_ERR(eventfp);
+               if (IS_ERR(eventfp)) {
+                       r = PTR_ERR(eventfp);
+                       break;
+               }
                if (eventfp != vq->kick) {
                        pollstop = filep = vq->kick;
                        pollstart = vq->kick = eventfp;

@@ -489,8 +491,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
                if (r < 0)
                        break;
                eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-               if (IS_ERR(eventfp))
-                       return PTR_ERR(eventfp);
+               if (IS_ERR(eventfp)) {
+                       r = PTR_ERR(eventfp);
+                       break;
+               }
                if (eventfp != vq->call) {
                        filep = vq->call;
                        ctx = vq->call_ctx;

@@ -505,8 +509,10 @@ static long vhost_set_vring(struct vhost_dev *d, int ioctl, void __user *argp)
                if (r < 0)
                        break;
                eventfp = f.fd == -1 ? NULL : eventfd_fget(f.fd);
-               if (IS_ERR(eventfp))
-                       return PTR_ERR(eventfp);
+               if (IS_ERR(eventfp)) {
+                       r = PTR_ERR(eventfp);
+                       break;
+               }
                if (eventfp != vq->error) {
                        filep = vq->error;
                        vq->error = eventfp;
--

1.7.0.18.g0d53a5
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html