From: Yufeng Wang <[email protected]>

The clear_user() call in VHOST_GET_FEATURES_ARRAY incorrectly starts
at argp, which is the beginning of the features array, overwriting the
data just written by copy_to_user(). It should start after the copied
elements at argp + copied * sizeof(u64) to only zero the trailing
unused space.

Use size_mul() for both the offset and length calculations so the
arithmetic stays consistent with the surrounding code and remains
overflow-safe.

Fixes: 333c515d1896 ("vhost-net: allow configuring extended features")
Signed-off-by: Yufeng Wang <[email protected]>

---
Changes in v2:
- Use size_mul() for the offset calculation as well, per review feedback.

Link to v1: 
https://lore.kernel.org/all/[email protected]/

Note:
Thank you for your review and suggestions.

I tried to add a switch in tools/virtio/vhost_net_test.c.
The switch is meant to use VHOST_GET_FEATURES_ARRAY and
VHOST_SET_FEATURES_ARRAY instead of the legacy versions.

However, when I ran `make virtio` in the tools directory,
the build failed with an error: missing asm/percpu_types.h.
I fixed that error, but then another error appeared.

Would it be acceptable to postpone the submission of
this test case until I have sorted out all the build
errors?
---
 drivers/vhost/net.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 77b59f49bddb..4b963dafa233 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1784,7 +1784,8 @@ static long vhost_net_ioctl(struct file *f, unsigned int 
ioctl,
                        return -EFAULT;
 
                /* Zero the trailing space provided by user-space, if any */
-               if (clear_user(argp, size_mul(count - copied, sizeof(u64))))
+               if (clear_user(argp + size_mul(copied, sizeof(u64)),
+                              size_mul(count - copied, sizeof(u64))))
                        return -EFAULT;
                return 0;
        case VHOST_SET_FEATURES_ARRAY:
-- 
2.34.1


Reply via email to