diff mbox series

[04/19] KVM: arm64: Push checks for 64bit registers into the low-level accessors

Message ID 20220706164304.1582687-5-maz@kernel.org (mailing list archive)
State New, archived
Headers show
Series KVM: arm64: vgic-v3 userspace access consolidation (and other goodies) | expand

Commit Message

Marc Zyngier July 6, 2022, 4:42 p.m. UTC
Make sure the check occurs on every paths where we can pick
a sysreg from userspace, including the GICv3 paths.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

Comments

Reiji Watanabe July 8, 2022, 6:13 a.m. UTC | #1
Hi Marc,

On Wed, Jul 6, 2022 at 9:43 AM Marc Zyngier <maz@kernel.org> wrote:
>
> Make sure the check occurs on every paths where we can pick
> a sysreg from userspace, including the GICv3 paths.
>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 0fbdb21a3600..89e7eddea937 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2656,6 +2656,10 @@ const struct sys_reg_desc *get_reg_by_id(u64 id,
>  {
>         struct sys_reg_params params;
>
> +       /* 64 bit is the only way */
> +       if (KVM_REG_SIZE(id) != sizeof(__u64))
> +               return NULL;

This doesn't seem to be necessary since the equivalent check
is done by index_to_params().

Thank you,
Reiji

> +
>         if (!index_to_params(id, &params))
>                 return NULL;
>
> @@ -2871,9 +2875,6 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
>         if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
>                 return demux_c15_get(reg->id, uaddr);
>
> -       if (KVM_REG_SIZE(reg->id) != sizeof(__u64))
> -               return -ENOENT;
> -
>         err = get_invariant_sys_reg(reg->id, uaddr);
>         if (err != -ENOENT)
>                 return err;
> @@ -2906,9 +2907,6 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
>         if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
>                 return demux_c15_set(reg->id, uaddr);
>
> -       if (KVM_REG_SIZE(reg->id) != sizeof(__u64))
> -               return -ENOENT;
> -
>         err = set_invariant_sys_reg(reg->id, uaddr);
>         if (err != -ENOENT)
>                 return err;
> --
> 2.34.1
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier July 8, 2022, 8:05 a.m. UTC | #2
On Fri, 08 Jul 2022 07:13:36 +0100,
Reiji Watanabe <reijiw@google.com> wrote:
> 
> Hi Marc,
> 
> On Wed, Jul 6, 2022 at 9:43 AM Marc Zyngier <maz@kernel.org> wrote:
> >
> > Make sure the check occurs on every paths where we can pick
> > a sysreg from userspace, including the GICv3 paths.
> >
> > Signed-off-by: Marc Zyngier <maz@kernel.org>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 0fbdb21a3600..89e7eddea937 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -2656,6 +2656,10 @@ const struct sys_reg_desc *get_reg_by_id(u64 id,
> >  {
> >         struct sys_reg_params params;
> >
> > +       /* 64 bit is the only way */
> > +       if (KVM_REG_SIZE(id) != sizeof(__u64))
> > +               return NULL;
> 
> This doesn't seem to be necessary since the equivalent check
> is done by index_to_params().

Ah, well spotted. Amusing how many times we check for the same thing!

I'll simplify the patch and amend the commit message.

Thanks,

	M.
diff mbox series

Patch

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 0fbdb21a3600..89e7eddea937 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2656,6 +2656,10 @@  const struct sys_reg_desc *get_reg_by_id(u64 id,
 {
 	struct sys_reg_params params;
 
+	/* 64 bit is the only way */
+	if (KVM_REG_SIZE(id) != sizeof(__u64))
+		return NULL;
+
 	if (!index_to_params(id, &params))
 		return NULL;
 
@@ -2871,9 +2875,6 @@  int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 	if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
 		return demux_c15_get(reg->id, uaddr);
 
-	if (KVM_REG_SIZE(reg->id) != sizeof(__u64))
-		return -ENOENT;
-
 	err = get_invariant_sys_reg(reg->id, uaddr);
 	if (err != -ENOENT)
 		return err;
@@ -2906,9 +2907,6 @@  int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 	if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
 		return demux_c15_set(reg->id, uaddr);
 
-	if (KVM_REG_SIZE(reg->id) != sizeof(__u64))
-		return -ENOENT;
-
 	err = set_invariant_sys_reg(reg->id, uaddr);
 	if (err != -ENOENT)
 		return err;