Re: [PATCH kvmtool 5/5] mmio: Sanitize addr and len

[Andre Przywara <andre.przywara@arm.com>]

On Tue, 18 Jan 2022 00:12:03 +0200
Martin Radev <martin.b.radev@gmail.com> wrote:

> This patch verifies that adding the addr and length arguments
> from an MMIO op do not overflow. This is necessary because the
> arguments are controlled by the VM. The length may be set to
> an arbitrary value by using the rep prefix.

Mmh, interesting, so does the kernel collate this into one
"giant" KVM_EXIT_MMIO with an arbitrary length? I wonder if there are
assumptions in the MMIO code of len never being bigger than say 16. On
ARM/ARM64 we probably never see len being bigger than 8 on those exits.

But the check is certainly fine anyway...

> Signed-off-by: Martin Radev <martin.b.radev@gmail.com>

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre

> ---
>  mmio.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/mmio.c b/mmio.c
> index a6dd3aa..04d2af6 100644
> --- a/mmio.c
> +++ b/mmio.c
> @@ -32,6 +32,10 @@ static struct mmio_mapping *mmio_search(struct rb_root *root, u64 addr, u64 len)
>  {
>  	struct rb_int_node *node;
>  
> +	/* If len is zero or if there's an overflow, the MMIO op is invalid. */
> +	if (len + addr <= addr)
> +		return NULL;
> +
>  	node = rb_int_search_range(root, addr, addr + len);
>  	if (node == NULL)
>  		return NULL;