vduse: fix memory corruption in vduse_dev_ioctl()
Subsystems (4)
PATCH v4 1/2 vduse: fix memory corruption in vduse_dev_ioctl()
The "config.offset" comes from the user. There needs to a check to prevent it being out of bounds. The "config.offset" and "dev->config_size" variables are both type u32. So if the offset if out of bounds then the "dev->config_size - config.offset" subtraction results in a very high u32 value. The out of bounds offset can result in memory corruption.
---
v2: fix reversed if statement
v3: fix vhost_vdpa_config_validate() as pointed out by Yongji Xie.
v4: split the vhost_vdpa_config_validate() change into a separate path
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/vdpa/vdpa_user/vduse_dev.c +2 -1
@@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,break ; ret = -EINVAL ; -if (config.length ==0 || +if (config.offset > dev->config_size || + config.length ==0 || config.length > dev->config_size - config.offset)break ;
Signature · 1 line
2.20.1
PATCH v4 2/2 vdpa: check that offsets are within bounds
In this function "c->off" is a u32 and "size" is a long. On 64bit systems if "c->off" is greater than "size" then "size - c->off" is a negative and we always return -E2BIG. But on 32bit systems the subtraction is type promoted to a high positive u32 value and basically any "c->len" is accepted.
---
v4: split into a separate patch
drivers/vhost/vdpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/vhost/vdpa.c +1 -1
@@ -197,7 +197,7 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa *v,struct vdpa_device *vdpa = v->vdpa;long size = vdpa->config->get_config_size (vdpa); -if (c->len ==0 ) +if (c->len ==0 || c->off > size)return -EINVAL ;if (c->len > size - c->off)
Signature · 1 line
2.20.1
Re: [PATCH 2/2 v4] vdpa: check that offsets are within bounds
Quoted 10 lines · Dan Carpenter
In this function "
c->off" is a u32 and "size" is a long. On 64bit systems if "c->off" is greater than "size" then "size -c->off" is a negative and we always return-E2BIG. But on 32bit systems the subtraction is type promoted to a high positive u32 value and basically any "c->len" is accepted.Fixes 4c8cf31885f6 ("vhost: introduce vDPA-based backend")Reported-by Xie Yongji <[email protected]>Signed-off-by Dan Carpenter <[email protected]>
Quoted 22 lines
---
v4: split into a separate patchdrivers/vhost/vdpa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/vhost/vdpa.c +1
-2@@ -197,7 +197,7 @@ static int vhost_vdpa_config_validate(struct vhost_vdpa *v,struct vdpa_device *vdpa = v->vdpa;long size = vdpa->config->get_config_size (vdpa); -if (c->len ==0 ) +if (c->len ==0 || c->off > size)return -EINVAL ;if (c->len > size - c->off) --2.20.1
Re: [PATCH 2/2 v4] vdpa: check that offsets are within bounds
In reply to Dan CarpenterQuoted 10 lines · Dan Carpenter
In this function "
c->off" is a u32 and "size" is a long. On 64bit systems if "c->off" is greater than "size" then "size -c->off" is a negative and we always return-E2BIG. But on 32bit systems the subtraction is type promoted to a high positive u32 value and basically any "c->len" is accepted.Fixes 4c8cf31885f6 ("vhost: introduce vDPA-based backend")Reported-by Xie Yongji <[email protected]>Signed-off-by Dan Carpenter <[email protected]>
Quoted 8 lines · Dan Carpenter
The "config.offset" comes from the user. There needs to a check to prevent it being out of bounds. The "config.offset" and "
dev->config_size" variables are both type u32. So if the offset if out of bounds then the "dev->config_size- config.offset" subtraction results in a very high u32 value. The out of bounds offset can result in memory corruption.
Quoted 6 lines
Fixes c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")Signed-off-by Dan Carpenter <[email protected]>---
v2: fix reversed if statement
v3: fixvhost_vdpa_config_validate()as pointed out by Yongji Xie.
v4: split thevhost_vdpa_config_validate()change into a separate path
Quoted 21 lines
drivers/vdpa/vdpa_user/vduse_dev.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)drivers/vdpa/vdpa_user/vduse_dev.c +2
-2@@ -975,7 +975,8 @@ static long vduse_dev_ioctl(struct file *file, unsigned int cmd,break ; ret = -EINVAL ; -if (config.length ==0 || +if (config.offset > dev->config_size || + config.length ==0 || config.length > dev->config_size - config.offset)break ; --2.20.1
Quoted 10 lines · Dan Carpenter
The "config.offset" comes from the user. There needs to a check to prevent it being out of bounds. The "config.offset" and "
dev->config_size" variables are both type u32. So if the offset if out of bounds then the "dev->config_size- config.offset" subtraction results in a very high u32 value. The out of bounds offset can result in memory corruption.Fixes c8a6153b6c59 ("vduse: Introduce VDUSE - vDPA Device in Userspace")Signed-off-by Dan Carpenter <[email protected]>