On 19/04/2023 09:06, Dan Carpenter wrote:
> The KVM_REG_SIZE() comes from the ioctl and it can be a power of two
> between 0-32768 but if it is more than sizeof(long) this will corrupt
> memory.
>
> Fixes: 99adb567632b ("KVM: arm/arm64: Add save/restore support for firmware workaround state")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Reviewed-by: Steven Price <steven.price@xxxxxxx>
Although there might be something to be said for rejecting anything
where KVM_REG_SIZE(reg->id) != sizeof(u64), as for smaller sizes the top
bits of val would be undefined which would require the code to mask the
top bits out to be safe. Given that all registers are currently u64 (and
I don't expect that to change), perhaps the below would be clearer?
if (KVM_REG_SIZE(reg->id) != sizeof(val))
return -EINVAL;
if (copy_from_user(&val, uaddr, sizeof(val)))
return -EFAULT;
Steve
> ---
> arch/arm64/kvm/hypercalls.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 2e16fc7b31bf..4f5767fcaca5 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -544,6 +544,8 @@ int kvm_arm_set_fw_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
> u64 val;
> int wa_level;
>
> + if (KVM_REG_SIZE(reg->id) > sizeof(val))
> + return -EINVAL;
> if (copy_from_user(&val, uaddr, KVM_REG_SIZE(reg->id)))
> return -EFAULT;
>
