* [PATCH] Fix vhost-user buffer over-read on ram hot-unplug
@ 2020-07-17 4:21 Raphael Norwitz
2020-07-23 4:08 ` Raphael Norwitz
2020-07-23 7:13 ` Marc-André Lureau
0 siblings, 2 replies; 3+ messages in thread
From: Raphael Norwitz @ 2020-07-17 4:21 UTC (permalink / raw)
To: qemu-devel, marcandre.lureau, mst; +Cc: Peter Turschmid, Raphael Norwitz
The VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS vhost-user protocol
feature introduced a shadow-table, used by the backend to dynamically
determine how a vdev's memory regions have changed since the last
vhost_user_set_mem_table() call. On hot-remove, a memmove() operation
is used to overwrite the removed shadow region descriptor(s). The size
parameter of this memmove was off by 1 such that if a VM with a backend
supporting the VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS filled it's
shadow-table (by performing the maximum number of supported hot-add
operatons) and attempted to remove the last region, Qemu would read an
out of bounds value and potentially crash.
This change fixes the memmove() bounds such that this erroneous read can
never happen.
Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com>
Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
---
hw/virtio/vhost-user.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 3123121..d7e2423 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -672,7 +672,7 @@ static int send_remove_regions(struct vhost_dev *dev,
memmove(&u->shadow_regions[shadow_reg_idx],
&u->shadow_regions[shadow_reg_idx + 1],
sizeof(struct vhost_memory_region) *
- (u->num_shadow_regions - shadow_reg_idx));
+ (u->num_shadow_regions - shadow_reg_idx - 1));
u->num_shadow_regions--;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] Fix vhost-user buffer over-read on ram hot-unplug
2020-07-17 4:21 [PATCH] Fix vhost-user buffer over-read on ram hot-unplug Raphael Norwitz
@ 2020-07-23 4:08 ` Raphael Norwitz
2020-07-23 7:13 ` Marc-André Lureau
1 sibling, 0 replies; 3+ messages in thread
From: Raphael Norwitz @ 2020-07-23 4:08 UTC (permalink / raw)
To: Raphael Norwitz
Cc: Marc-André Lureau, QEMU, Peter Turschmid, Michael S. Tsirkin
ping
On Thu, Jul 16, 2020 at 10:21 PM Raphael Norwitz
<raphael.norwitz@nutanix.com> wrote:
>
> The VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS vhost-user protocol
> feature introduced a shadow-table, used by the backend to dynamically
> determine how a vdev's memory regions have changed since the last
> vhost_user_set_mem_table() call. On hot-remove, a memmove() operation
> is used to overwrite the removed shadow region descriptor(s). The size
> parameter of this memmove was off by 1 such that if a VM with a backend
> supporting the VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS filled it's
> shadow-table (by performing the maximum number of supported hot-add
> operatons) and attempted to remove the last region, Qemu would read an
> out of bounds value and potentially crash.
>
> This change fixes the memmove() bounds such that this erroneous read can
> never happen.
>
> Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com>
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
> ---
> hw/virtio/vhost-user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 3123121..d7e2423 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -672,7 +672,7 @@ static int send_remove_regions(struct vhost_dev *dev,
> memmove(&u->shadow_regions[shadow_reg_idx],
> &u->shadow_regions[shadow_reg_idx + 1],
> sizeof(struct vhost_memory_region) *
> - (u->num_shadow_regions - shadow_reg_idx));
> + (u->num_shadow_regions - shadow_reg_idx - 1));
> u->num_shadow_regions--;
> }
>
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix vhost-user buffer over-read on ram hot-unplug
2020-07-17 4:21 [PATCH] Fix vhost-user buffer over-read on ram hot-unplug Raphael Norwitz
2020-07-23 4:08 ` Raphael Norwitz
@ 2020-07-23 7:13 ` Marc-André Lureau
1 sibling, 0 replies; 3+ messages in thread
From: Marc-André Lureau @ 2020-07-23 7:13 UTC (permalink / raw)
To: Raphael Norwitz; +Cc: Peter Turschmid, qemu-devel, Michael Tsirkin
On Fri, Jul 17, 2020 at 8:21 AM Raphael Norwitz
<raphael.norwitz@nutanix.com> wrote:
>
> The VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS vhost-user protocol
> feature introduced a shadow-table, used by the backend to dynamically
> determine how a vdev's memory regions have changed since the last
> vhost_user_set_mem_table() call. On hot-remove, a memmove() operation
> is used to overwrite the removed shadow region descriptor(s). The size
> parameter of this memmove was off by 1 such that if a VM with a backend
> supporting the VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS filled it's
> shadow-table (by performing the maximum number of supported hot-add
> operatons) and attempted to remove the last region, Qemu would read an
> out of bounds value and potentially crash.
>
> This change fixes the memmove() bounds such that this erroneous read can
> never happen.
>
> Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com>
> Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
Fixes: f1aeb14b0809 ("Transmit vhost-user memory regions individually")
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/virtio/vhost-user.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
> index 3123121..d7e2423 100644
> --- a/hw/virtio/vhost-user.c
> +++ b/hw/virtio/vhost-user.c
> @@ -672,7 +672,7 @@ static int send_remove_regions(struct vhost_dev *dev,
> memmove(&u->shadow_regions[shadow_reg_idx],
> &u->shadow_regions[shadow_reg_idx + 1],
> sizeof(struct vhost_memory_region) *
> - (u->num_shadow_regions - shadow_reg_idx));
> + (u->num_shadow_regions - shadow_reg_idx - 1));
> u->num_shadow_regions--;
> }
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-23 7:22 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-07-17 4:21 [PATCH] Fix vhost-user buffer over-read on ram hot-unplug Raphael Norwitz
2020-07-23 4:08 ` Raphael Norwitz
2020-07-23 7:13 ` Marc-André Lureau
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).