vhost_vdpa: Return -EFUALT if copy_from_user() fails

PATCH In review Dan Carpenter 3 participants 6 messages Oct 23, 2020 – Nov 18, 2020

Subsystems (6)

Networking Network drivers KVM Kernel janitors +2 more subsystems

lore

Share & tools

lore

Dan Carpenter

author

2020-10-23 12:08 UTC #0 lore raw

PATCH 1/1 vhost_vdpa: Return -EFUALT if copy_from_user() fails

The copy_to/from_user() functions return the number of bytes which we weren't able to copy but the ioctl should return -EFAULT if they fail.

Fixes a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")

Signed-off-by Dan Carpenter <[email protected]>

---
 drivers/vhost/vdpa.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/vhost/vdpa.c +5 -5

index 62a9bb0efc55..c94a97b6bd6d 100644
@@ -428,12 +428,11 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 	void __user *argp = (void __user *)arg;
 	u64 __user *featurep = argp;
 	u64 features;
-	long r;
+	long r = 0;
 
 	if (cmd = VHOST_SET_BACKEND_FEATURES) {
-		r = copy_from_user(&features, featurep, sizeof(features));
-		if (r)
-			return r;
+		if (copy_from_user(&features, featurep, sizeof(features)))
+			return -EFAULT;
 		if (features & ~VHOST_VDPA_BACKEND_FEATURES)
 			return -EOPNOTSUPP;
 		vhost_set_backend_features(&v->vdev, features);

@@ -476,7 +475,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 		break;
 	case VHOST_GET_BACKEND_FEATURES:
 		features = VHOST_VDPA_BACKEND_FEATURES;
-		r = copy_to_user(featurep, &features, sizeof(features));
+		if (copy_to_user(featurep, &features, sizeof(features)))
+			r = -EFAULT;
 		break;
 	default:
 		r = vhost_dev_ioctl(&v->vdev, cmd, argp);

Michael S. Tsirkin

maintainer Acked-by on v1 1/1

2020-10-23 15:34 UTC #1 lore raw

Quoted 5 lines · Dan Carpenter

The copy_to/from_user() functions return the number of bytes which we weren't able to copy but the ioctl should return -EFAULT if they fail.
Fixes a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")
Signed-off-by Dan Carpenter <[email protected]>

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

Needed for stable I guess.

Quoted 34 lines

---
 drivers/vhost/vdpa.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/vhost/vdpa.c +5 -5

index 62a9bb0efc55..c94a97b6bd6d 100644
@@ -428,12 +428,11 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 	void __user *argp = (void __user *)arg;
 	u64 __user *featurep = argp;
 	u64 features;
-	long r;
+	long r = 0;
 
 	if (cmd = VHOST_SET_BACKEND_FEATURES) {
-		r = copy_from_user(&features, featurep, sizeof(features));
-		if (r)
-			return r;
+		if (copy_from_user(&features, featurep, sizeof(features)))
+			return -EFAULT;
 		if (features & ~VHOST_VDPA_BACKEND_FEATURES)
 			return -EOPNOTSUPP;
 		vhost_set_backend_features(&v->vdev, features);

@@ -476,7 +475,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
 		break;
 	case VHOST_GET_BACKEND_FEATURES:
 		features = VHOST_VDPA_BACKEND_FEATURES;
-		r = copy_to_user(featurep, &features, sizeof(features));
+		if (copy_to_user(featurep, &features, sizeof(features)))
+			r = -EFAULT;
 		break;
 	default:
 		r = vhost_dev_ioctl(&v->vdev, cmd, argp);

Jason Wang

Acked-by

2020-10-26 02:59 UTC #2 lore raw

Quoted 8 lines · Michael S. Tsirkin

Quoted 5 lines · Dan Carpenter
The copy_to/from_user() functions return the number of bytes which we weren't able to copy but the ioctl should return -EFAULT if they fail.
Fixes a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Michael S. Tsirkin <[email protected]>
Needed for stable I guess.

Agree.

Acked-by Jason Wang <[email protected]>

Quoted 34 lines

---
  drivers/vhost/vdpa.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)

drivers/vhost/vdpa.c +5 -5

index 62a9bb0efc55..c94a97b6bd6d 100644
@@ -428,12 +428,11 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
  	void __user *argp = (void __user *)arg;
  	u64 __user *featurep = argp;
  	u64 features;
-	long r;
+	long r = 0;
 
  	if (cmd = VHOST_SET_BACKEND_FEATURES) {
-		r = copy_from_user(&features, featurep, sizeof(features));
-		if (r)
-			return r;
+		if (copy_from_user(&features, featurep, sizeof(features)))
+			return -EFAULT;
  		if (features & ~VHOST_VDPA_BACKEND_FEATURES)
  			return -EOPNOTSUPP;
  		vhost_set_backend_features(&v->vdev, features);

@@ -476,7 +475,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
  		break;
  	case VHOST_GET_BACKEND_FEATURES:
  		features = VHOST_VDPA_BACKEND_FEATURES;
-		r = copy_to_user(featurep, &features, sizeof(features));
+		if (copy_to_user(featurep, &features, sizeof(features)))
+			r = -EFAULT;
  		break;
  	default:
  		r = vhost_dev_ioctl(&v->vdev, cmd, argp);

Jason Wang

2020-11-18 06:08 UTC #3 lore raw

Quoted 8 lines · Jason Wang

Quoted 6 lines · Michael S. Tsirkin
Quoted 5 lines · Dan Carpenter
The copy_to/from_user() functions return the number of bytes which we weren't able to copy but the ioctl should return -EFAULT if they fail.
Fixes a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")
Signed-off-by Dan Carpenter <[email protected]>

Quoted 7 lines

Quoted 2 lines
Acked-by Michael S. Tsirkin <[email protected]>
Needed for stable I guess.
Agree.
Acked-by Jason Wang <[email protected]>

Hi Michael.

I don't see this in your tree, please consider to merge.

Thanks

Michael S. Tsirkin

maintainer

2020-11-18 08:59 UTC #4 lore raw

Quoted 16 lines · Jason Wang

Quoted 14 lines · Jason Wang
Quoted 8 lines · Michael S. Tsirkin
Quoted 5 lines · Dan Carpenter
The copy_to/from_user() functions return the number of bytes which we weren't able to copy but the ioctl should return -EFAULT if they fail.
Fixes a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Michael S. Tsirkin <[email protected]>
Needed for stable I guess.
Agree.

Quoted 9 lines

Quoted 1 line
Acked-by Jason Wang <[email protected]>
Hi Michael.
I don't see this in your tree, please consider to merge.
Thanks

I do see it there:

commit 7922460e33c81f41e0d2421417228b32e6fdbe94

Author: Dan Carpenter <[email protected]>
Date:   Fri Oct 23 15:08:53 2020 +0300

vhost_vdpa: Return -EFAULT if copy_from_user() fails

the reason you can't find it is probably because I fixed up a typo in the subject.

Signature · 1 line

MST

Jason Wang

2020-11-18 09:39 UTC #5 lore raw

Quoted 24 lines · Michael S. Tsirkin

Quoted 13 lines · Jason Wang
Quoted 12 lines · Jason Wang
Quoted 8 lines · Michael S. Tsirkin
Quoted 5 lines · Dan Carpenter
The copy_to/from_user() functions return the number of bytes which we weren't able to copy but the ioctl should return -EFAULT if they fail.
Fixes a127c5bbb6a8 ("vhost-vdpa: fix backend feature ioctls")
Signed-off-by Dan Carpenter <[email protected]>
Acked-by Michael S. Tsirkin <[email protected]>
Needed for stable I guess.
Agree.
Quoted 8 lines
Quoted 1 line
Acked-by Jason Wang <[email protected]>
Hi Michael.
I don't see this in your tree, please consider to merge.
Thanks
I do see it there:

Quoted 8 lines

commit 7922460e33c81f41e0d2421417228b32e6fdbe94
Author: Dan Carpenter <[email protected]>
Date:   Fri Oct 23 15:08:53 2020 +0300
vhost_vdpa: Return -EFAULT if copy_from_user() fails
the reason you can't find it is probably because I fixed up a typo in the subject.

I see that.

Thanks

Quoted 2 lines