From: Ricardo Koller <ricarkol@google.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: kvm@vger.kernel.org, maz@kernel.org, pshier@google.com,
Paolo Bonzini <pbonzini@redhat.com>,
shuah@kernel.org, kvmarm@lists.cs.columbia.edu
Subject: Re: [PATCH 1/2] KVM: arm64: vgic: check redist region is not above the VM IPA size
Date: Thu, 9 Sep 2021 09:47:13 -0700 [thread overview]
Message-ID: <YTo6kX7jGeR3XvPg@google.com> (raw)
In-Reply-To: <b368e9cf-ec28-1768-edf9-dfdc7fa108f8@arm.com>
On Thu, Sep 09, 2021 at 11:20:15AM +0100, Alexandru Elisei wrote:
> Hi Ricardo,
>
> On 9/8/21 10:03 PM, Ricardo Koller wrote:
> > Extend vgic_v3_check_base() to verify that the redistributor regions
> > don't go above the VM-specified IPA size (phys_size). This can happen
> > when using the legacy KVM_VGIC_V3_ADDR_TYPE_REDIST attribute with:
> >
> > base + size > phys_size AND base < phys_size
> >
> > vgic_v3_check_base() is used to check the redist regions bases when
> > setting them (with the vcpus added so far) and when attempting the first
> > vcpu-run.
> >
> > Signed-off-by: Ricardo Koller <ricarkol@google.com>
> > ---
> > arch/arm64/kvm/vgic/vgic-v3.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index 66004f61cd83..5afd9f6f68f6 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -512,6 +512,10 @@ bool vgic_v3_check_base(struct kvm *kvm)
> > if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) <
> > rdreg->base)
> > return false;
> > +
> > + if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) >
> > + kvm_phys_size(kvm))
> > + return false;
>
> Looks to me like this same check (and the overflow one before it) is done when
> adding a new Redistributor region in kvm_vgic_addr() -> vgic_v3_set_redist_base()
> -> vgic_v3_alloc_redist_region() -> vgic_check_ioaddr(). As far as I can tell,
> kvm_vgic_addr() handles both ways of setting the Redistributor address.
>
> Without this patch, did you manage to set a base address such that base + size >
> kvm_phys_size()?
>
Yes, with the KVM_VGIC_V3_ADDR_TYPE_REDIST legacy API. The easiest way
to get to this situation is with the selftest in patch 2. I then tried
an extra experiment: map the first redistributor, run the first vcpu,
and access the redist from inside the guest. KVM didn't complain in any
of these steps.
Thanks,
Ricardo
> Thanks,
>
> Alex
>
> > }
> >
> > if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base))
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: Ricardo Koller <ricarkol@google.com>
To: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: kvm@vger.kernel.org, maz@kernel.org,
kvmarm@lists.cs.columbia.edu, drjones@redhat.com,
eric.auger@redhat.com, Paolo Bonzini <pbonzini@redhat.com>,
oupton@google.com, james.morse@arm.com, suzuki.poulose@arm.com,
shuah@kernel.org, jingzhangos@google.com, pshier@google.com,
rananta@google.com, reijiw@google.com
Subject: Re: [PATCH 1/2] KVM: arm64: vgic: check redist region is not above the VM IPA size
Date: Thu, 9 Sep 2021 09:47:13 -0700 [thread overview]
Message-ID: <YTo6kX7jGeR3XvPg@google.com> (raw)
In-Reply-To: <b368e9cf-ec28-1768-edf9-dfdc7fa108f8@arm.com>
On Thu, Sep 09, 2021 at 11:20:15AM +0100, Alexandru Elisei wrote:
> Hi Ricardo,
>
> On 9/8/21 10:03 PM, Ricardo Koller wrote:
> > Extend vgic_v3_check_base() to verify that the redistributor regions
> > don't go above the VM-specified IPA size (phys_size). This can happen
> > when using the legacy KVM_VGIC_V3_ADDR_TYPE_REDIST attribute with:
> >
> > base + size > phys_size AND base < phys_size
> >
> > vgic_v3_check_base() is used to check the redist regions bases when
> > setting them (with the vcpus added so far) and when attempting the first
> > vcpu-run.
> >
> > Signed-off-by: Ricardo Koller <ricarkol@google.com>
> > ---
> > arch/arm64/kvm/vgic/vgic-v3.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
> > index 66004f61cd83..5afd9f6f68f6 100644
> > --- a/arch/arm64/kvm/vgic/vgic-v3.c
> > +++ b/arch/arm64/kvm/vgic/vgic-v3.c
> > @@ -512,6 +512,10 @@ bool vgic_v3_check_base(struct kvm *kvm)
> > if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) <
> > rdreg->base)
> > return false;
> > +
> > + if (rdreg->base + vgic_v3_rd_region_size(kvm, rdreg) >
> > + kvm_phys_size(kvm))
> > + return false;
>
> Looks to me like this same check (and the overflow one before it) is done when
> adding a new Redistributor region in kvm_vgic_addr() -> vgic_v3_set_redist_base()
> -> vgic_v3_alloc_redist_region() -> vgic_check_ioaddr(). As far as I can tell,
> kvm_vgic_addr() handles both ways of setting the Redistributor address.
>
> Without this patch, did you manage to set a base address such that base + size >
> kvm_phys_size()?
>
Yes, with the KVM_VGIC_V3_ADDR_TYPE_REDIST legacy API. The easiest way
to get to this situation is with the selftest in patch 2. I then tried
an extra experiment: map the first redistributor, run the first vcpu,
and access the redist from inside the guest. KVM didn't complain in any
of these steps.
Thanks,
Ricardo
> Thanks,
>
> Alex
>
> > }
> >
> > if (IS_VGIC_ADDR_UNDEF(d->vgic_dist_base))
next prev parent reply other threads:[~2021-09-09 16:47 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-08 21:03 [PATCH 0/2] KVM: arm64: vgic-v3: Missing check for redist region above the VM IPA size Ricardo Koller
2021-09-08 21:03 ` Ricardo Koller
2021-09-08 21:03 ` [PATCH 1/2] KVM: arm64: vgic: check redist region is not " Ricardo Koller
2021-09-08 21:03 ` Ricardo Koller
2021-09-08 21:32 ` Oliver Upton
2021-09-08 21:32 ` Oliver Upton
2021-09-08 21:50 ` Ricardo Koller
2021-09-08 21:50 ` Ricardo Koller
2021-09-08 22:00 ` Oliver Upton
2021-09-08 22:00 ` Oliver Upton
2021-09-09 10:20 ` Alexandru Elisei
2021-09-09 10:20 ` Alexandru Elisei
2021-09-09 14:43 ` Eric Auger
2021-09-09 14:43 ` Eric Auger
2021-09-09 16:47 ` Ricardo Koller [this message]
2021-09-09 16:47 ` Ricardo Koller
2021-09-10 8:28 ` Alexandru Elisei
2021-09-10 8:28 ` Alexandru Elisei
2021-09-10 8:42 ` Eric Auger
2021-09-10 8:42 ` Eric Auger
2021-09-10 19:32 ` Ricardo Koller
2021-09-10 19:32 ` Ricardo Koller
2021-09-13 8:51 ` Eric Auger
2021-09-13 8:51 ` Eric Auger
2021-09-13 10:15 ` Alexandru Elisei
2021-09-14 3:20 ` Ricardo Koller
2021-09-14 3:20 ` Ricardo Koller
2021-09-14 11:00 ` Alexandru Elisei
2021-09-20 21:01 ` Ricardo Koller
2021-09-20 21:01 ` Ricardo Koller
2021-09-08 21:03 ` [PATCH 2/2] KVM: arm64: selftests: test for vgic redist " Ricardo Koller
2021-09-08 21:03 ` Ricardo Koller
2021-09-09 13:54 ` Eric Auger
2021-09-09 13:54 ` Eric Auger
2021-09-09 18:22 ` Ricardo Koller
2021-09-09 18:22 ` Ricardo Koller
2021-09-10 7:12 ` Eric Auger
2021-09-10 7:12 ` Eric Auger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YTo6kX7jGeR3XvPg@google.com \
--to=ricarkol@google.com \
--cc=alexandru.elisei@arm.com \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=maz@kernel.org \
--cc=pbonzini@redhat.com \
--cc=pshier@google.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.