diff mbox series

vduse: Fix off by one in vduse_dev_mmap()

Message ID e26476e0-68ae-412d-a5d9-4996bc30d038@moroto.mountain
State New
Headers
Series vduse: Fix off by one in vduse_dev_mmap() |

Commit Message

Dan Carpenter Feb. 27, 2024, 3:21 p.m. UTC 
  The dev->vqs[] array has "dev->vq_num" elements.  It's allocated in
vduse_dev_init_vqs().  Thus, this > comparison needs to be >= to avoid
reading one element beyond the end of the array.

Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/vdpa/vdpa_user/vduse_dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Dan Carpenter Feb. 28, 2024, 5:42 p.m. UTC | #1 
On Tue, Feb 27, 2024 at 10:48:49AM -0500, Michael S. Tsirkin wrote:
> On Tue, Feb 27, 2024 at 06:21:46PM +0300, Dan Carpenter wrote:
> > The dev->vqs[] array has "dev->vq_num" elements.  It's allocated in
> > vduse_dev_init_vqs().  Thus, this > comparison needs to be >= to avoid
> > reading one element beyond the end of the array.
> > 
> > Fixes: 316ecd1346b0 ("vduse: Add file operation for mmap")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> 
> Oh wow and does this not come from userspace? If yes we
> need the speculation magic macro when using the index, do we not?
> 

Yes, it does come from userspace.

To be honest, I'm not sure about speculation.  The similar places in
this file protect against speculation so, probably?  I'll resend the
patch.

regards,
dan carpenter
diff mbox series

Patch

diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
index b7a1fb88c506..9150c8281953 100644
--- a/drivers/vdpa/vdpa_user/vduse_dev.c
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -1532,7 +1532,7 @@  static int vduse_dev_mmap(struct file *file, struct vm_area_struct *vma)
 	if ((vma->vm_flags & VM_SHARED) == 0)
 		return -EINVAL;
 
-	if (index > dev->vq_num)
+	if (index >= dev->vq_num)
 		return -EINVAL;
 
 	vq = dev->vqs[index];