[PATCH] KVM: x86: use array_index_nospec with indices that come from guest

[PATCH] KVM: x86: use array_index_nospec with indices that come from guest

[Greg Kroah-Hartman]

From: Thijs Raymakers <thijs@raymakers.nl>

min and dest_id are guest-controlled indices. Using array_index_nospec()
after the bounds checks clamps these values to mitigate speculative
execution side-channels.

Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
Cc: stable <stable@kernel.org>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/x86/kvm/lapic.c | 2 ++
 arch/x86/kvm/x86.c   | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 73418dc0ebb2..e10d6ad236c9 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -852,6 +852,8 @@ static int __pv_send_ipi(unsigned long *ipi_bitmap, struct kvm_apic_map *map,
 	if (min > map->max_apic_id)
 		return 0;
 
+	min = array_index_nospec(min, map->max_apic_id);
+
 	for_each_set_bit(i, ipi_bitmap,
 		min((u32)BITS_PER_LONG, (map->max_apic_id - min + 1))) {
 		if (map->phys_map[min + i]) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 93636f77c42d..872e43defa67 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10051,8 +10051,11 @@ static void kvm_sched_yield(struct kvm_vcpu *vcpu, unsigned long dest_id)
 	rcu_read_lock();
 	map = rcu_dereference(vcpu->kvm->arch.apic_map);
 
-	if (likely(map) && dest_id <= map->max_apic_id && map->phys_map[dest_id])
-		target = map->phys_map[dest_id]->vcpu;
+	if (likely(map) && dest_id <= map->max_apic_id) {
+		dest_id = array_index_nospec(dest_id, map->max_apic_id);
+		if (map->phys_map[dest_id])
+			target = map->phys_map[dest_id]->vcpu;
+	}
 
 	rcu_read_unlock();
 
-- 
2.50.1

Re: [PATCH] KVM: x86: use array_index_nospec with indices that come from guest

[Sean Christopherson]

On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> From: Thijs Raymakers <thijs@raymakers.nl>
> 
> min and dest_id are guest-controlled indices. Using array_index_nospec()
> after the bounds checks clamps these values to mitigate speculative
> execution side-channels.
> 
> Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> Cc: stable <stable@kernel.org>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  arch/x86/kvm/lapic.c | 2 ++
>  arch/x86/kvm/x86.c   | 7 +++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 73418dc0ebb2..e10d6ad236c9 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -852,6 +852,8 @@ static int __pv_send_ipi(unsigned long *ipi_bitmap, struct kvm_apic_map *map,
>  	if (min > map->max_apic_id)
>  		return 0;
>  
> +	min = array_index_nospec(min, map->max_apic_id);

This is wrong, max_apic_id is inclusive, whereas array_index_nospec() takes a
size/length as the second argument.  I.e. this needs to be:

	min = array_index_nospec(min, map->max_apic_id + 1);

> +
>  	for_each_set_bit(i, ipi_bitmap,
>  		min((u32)BITS_PER_LONG, (map->max_apic_id - min + 1))) {
>  		if (map->phys_map[min + i]) {
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 93636f77c42d..872e43defa67 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10051,8 +10051,11 @@ static void kvm_sched_yield(struct kvm_vcpu *vcpu, unsigned long dest_id)
>  	rcu_read_lock();
>  	map = rcu_dereference(vcpu->kvm->arch.apic_map);
>  
> -	if (likely(map) && dest_id <= map->max_apic_id && map->phys_map[dest_id])
> -		target = map->phys_map[dest_id]->vcpu;
> +	if (likely(map) && dest_id <= map->max_apic_id) {
> +		dest_id = array_index_nospec(dest_id, map->max_apic_id);

Same thing here.

> +		if (map->phys_map[dest_id])
> +			target = map->phys_map[dest_id]->vcpu;
> +	}
>  
>  	rcu_read_unlock();
>  
> -- 
> 2.50.1
> 

[PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Thijs Raymakers]

min and dest_id are guest-controlled indices. Using array_index_nospec()
after the bounds checks clamps these values to mitigate speculative execution
side-channels.

Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
Cc: stable <stable@kernel.org>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/x86/kvm/lapic.c | 2 ++
 arch/x86/kvm/x86.c   | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 73418dc0ebb2..0725d2cae742 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -852,6 +852,8 @@ static int __pv_send_ipi(unsigned long *ipi_bitmap, struct kvm_apic_map *map,
 	if (min > map->max_apic_id)
 		return 0;
 
+	min = array_index_nospec(min, map->max_apic_id + 1);
+
 	for_each_set_bit(i, ipi_bitmap,
 		min((u32)BITS_PER_LONG, (map->max_apic_id - min + 1))) {
 		if (map->phys_map[min + i]) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 93636f77c42d..43b63f1d1594 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10051,8 +10051,11 @@ static void kvm_sched_yield(struct kvm_vcpu *vcpu, unsigned long dest_id)
 	rcu_read_lock();
 	map = rcu_dereference(vcpu->kvm->arch.apic_map);
 
-	if (likely(map) && dest_id <= map->max_apic_id && map->phys_map[dest_id])
-		target = map->phys_map[dest_id]->vcpu;
+	if (likely(map) && dest_id <= map->max_apic_id) {
+		dest_id = array_index_nospec(dest_id, map->max_apic_id + 1);
+		if (map->phys_map[dest_id])
+			target = map->phys_map[dest_id]->vcpu;
+	}
 
 	rcu_read_unlock();
 
-- 
2.50.1

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Greg Kroah-Hartman]

On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> min and dest_id are guest-controlled indices. Using array_index_nospec()
> after the bounds checks clamps these values to mitigate speculative execution
> side-channels.
> 
> Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> Cc: stable <stable@kernel.org>
> Cc: Sean Christopherson <seanjc@google.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Nit, you shouldn't have added my signed off on a new version, but that's
ok, I'm fine with it.

> ---
>  arch/x86/kvm/lapic.c | 2 ++
>  arch/x86/kvm/x86.c   | 7 +++++--
>  2 files changed, 7 insertions(+), 2 deletions(-)

You also forgot to say what changed down here.

Don't know how strict the KVM maintainers are, I know I require these
things fixed up...

thanks,

greg k-h

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Sean Christopherson]

On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> > min and dest_id are guest-controlled indices. Using array_index_nospec()
> > after the bounds checks clamps these values to mitigate speculative execution
> > side-channels.
> > 
> > Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> > Cc: stable <stable@kernel.org>
> > Cc: Sean Christopherson <seanjc@google.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Nit, you shouldn't have added my signed off on a new version, but that's
> ok, I'm fine with it.

Want me to keep your SoB when applying, or drop it?

> > ---
> >  arch/x86/kvm/lapic.c | 2 ++
> >  arch/x86/kvm/x86.c   | 7 +++++--
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> You also forgot to say what changed down here.
> 
> Don't know how strict the KVM maintainers are, I know I require these
> things fixed up...

I require the same things, but I also don't mind doing fixup when applying if
that's the path of least resistance (and it's not a recurring problem).

I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
b4, and often confuses me as well.

But for this, I don't see any reason to send a v3.

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Greg Kroah-Hartman]

On Thu, Jul 24, 2025 at 12:04:15PM -0700, Sean Christopherson wrote:
> On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> > On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> > > min and dest_id are guest-controlled indices. Using array_index_nospec()
> > > after the bounds checks clamps these values to mitigate speculative execution
> > > side-channels.
> > > 
> > > Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> > > Cc: stable <stable@kernel.org>
> > > Cc: Sean Christopherson <seanjc@google.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > Nit, you shouldn't have added my signed off on a new version, but that's
> > ok, I'm fine with it.
> 
> Want me to keep your SoB when applying, or drop it?

Keep it please, I was just letting Thijs know.

> > > ---
> > >  arch/x86/kvm/lapic.c | 2 ++
> > >  arch/x86/kvm/x86.c   | 7 +++++--
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > You also forgot to say what changed down here.
> > 
> > Don't know how strict the KVM maintainers are, I know I require these
> > things fixed up...
> 
> I require the same things, but I also don't mind doing fixup when applying if
> that's the path of least resistance (and it's not a recurring problem).
> 
> I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
> b4, and often confuses me as well.
> 
> But for this, I don't see any reason to send a v3.

That's great, thanks.

greg k-h

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Thijs Raymakers]

On 7/25/25 6:42 AM, Greg Kroah-Hartman wrote:
> On Thu, Jul 24, 2025 at 12:04:15PM -0700, Sean Christopherson wrote:
>> On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
>>> On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
>>>> min and dest_id are guest-controlled indices. Using array_index_nospec()
>>>> after the bounds checks clamps these values to mitigate speculative execution
>>>> side-channels.
>>>>
>>>> Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
>>>> Cc: stable <stable@kernel.org>
>>>> Cc: Sean Christopherson <seanjc@google.com>
>>>> Cc: Paolo Bonzini <pbonzini@redhat.com>
>>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Nit, you shouldn't have added my signed off on a new version, but that's
>>> ok, I'm fine with it.
>> Want me to keep your SoB when applying, or drop it?
> Keep it please, I was just letting Thijs know.
Sorry about that. I was not entirely sure whether tags like Signed-Off 
should
be kept or removed in a new revision. Thanks for the feedback.
>>>> ---
>>>>   arch/x86/kvm/lapic.c | 2 ++
>>>>   arch/x86/kvm/x86.c   | 7 +++++--
>>>>   2 files changed, 7 insertions(+), 2 deletions(-)
>>> You also forgot to say what changed down here.
>>>
>>> Don't know how strict the KVM maintainers are, I know I require these
>>> things fixed up...
>> I require the same things, but I also don't mind doing fixup when applying if
>> that's the path of least resistance (and it's not a recurring problem).

Changes in v2:
- As noted by Sean Christopherson, max_apic_id is inclusive but array_index_nospec is not.
   v2 adds one to the array_index_nospec size so the bounds do include max_apic_id
- Link to v1: https://lore.kernel.org/kvm/2025072540-eggbeater-crate-50af@gregkh/T/#u

>> I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
>> b4, and often confuses me as well.

Noted, will not do it like that next time.

>>
>> But for this, I don't see any reason to send a v3.
> That's great, thanks.

Thanks. I'm fairly new to the process of submitting patches over email, 
so apologies for my mistakes. Thank you for your patience and feedback, 
it is much appreciated. If you do prefer a v3 that does include the 
change log, just let me know.

- Thijs

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Greg Kroah-Hartman]

On Thu, Jul 24, 2025 at 12:04:15PM -0700, Sean Christopherson wrote:
> On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> > On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> > > min and dest_id are guest-controlled indices. Using array_index_nospec()
> > > after the bounds checks clamps these values to mitigate speculative execution
> > > side-channels.
> > > 
> > > Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> > > Cc: stable <stable@kernel.org>
> > > Cc: Sean Christopherson <seanjc@google.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > 
> > Nit, you shouldn't have added my signed off on a new version, but that's
> > ok, I'm fine with it.
> 
> Want me to keep your SoB when applying, or drop it?
> 
> > > ---
> > >  arch/x86/kvm/lapic.c | 2 ++
> > >  arch/x86/kvm/x86.c   | 7 +++++--
> > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > You also forgot to say what changed down here.
> > 
> > Don't know how strict the KVM maintainers are, I know I require these
> > things fixed up...
> 
> I require the same things, but I also don't mind doing fixup when applying if
> that's the path of least resistance (and it's not a recurring problem).
> 
> I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
> b4, and often confuses me as well.
> 
> But for this, I don't see any reason to send a v3.

Any status on this?  I don't see it in linux-next at all, nor in
6.17-rc1

thanks,

greg k-h

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Sean Christopherson]

On Mon, Aug 11, 2025, Greg Kroah-Hartman wrote:
> On Thu, Jul 24, 2025 at 12:04:15PM -0700, Sean Christopherson wrote:
> > On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> > > On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> > > > min and dest_id are guest-controlled indices. Using array_index_nospec()
> > > > after the bounds checks clamps these values to mitigate speculative execution
> > > > side-channels.
> > > > 
> > > > Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> > > > Cc: stable <stable@kernel.org>
> > > > Cc: Sean Christopherson <seanjc@google.com>
> > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > 
> > > Nit, you shouldn't have added my signed off on a new version, but that's
> > > ok, I'm fine with it.
> > 
> > Want me to keep your SoB when applying, or drop it?
> > 
> > > > ---
> > > >  arch/x86/kvm/lapic.c | 2 ++
> > > >  arch/x86/kvm/x86.c   | 7 +++++--
> > > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > > 
> > > You also forgot to say what changed down here.
> > > 
> > > Don't know how strict the KVM maintainers are, I know I require these
> > > things fixed up...
> > 
> > I require the same things, but I also don't mind doing fixup when applying if
> > that's the path of least resistance (and it's not a recurring problem).
> > 
> > I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
> > b4, and often confuses me as well.
> > 
> > But for this, I don't see any reason to send a v3.
> 
> Any status on this?  I don't see it in linux-next at all, nor in
> 6.17-rc1

I'll get it applied and sent along to Paolo/Linus this week.

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Greg Kroah-Hartman]

On Mon, Aug 11, 2025 at 07:35:49AM -0700, Sean Christopherson wrote:
> On Mon, Aug 11, 2025, Greg Kroah-Hartman wrote:
> > On Thu, Jul 24, 2025 at 12:04:15PM -0700, Sean Christopherson wrote:
> > > On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> > > > On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> > > > > min and dest_id are guest-controlled indices. Using array_index_nospec()
> > > > > after the bounds checks clamps these values to mitigate speculative execution
> > > > > side-channels.
> > > > > 
> > > > > Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> > > > > Cc: stable <stable@kernel.org>
> > > > > Cc: Sean Christopherson <seanjc@google.com>
> > > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > 
> > > > Nit, you shouldn't have added my signed off on a new version, but that's
> > > > ok, I'm fine with it.
> > > 
> > > Want me to keep your SoB when applying, or drop it?
> > > 
> > > > > ---
> > > > >  arch/x86/kvm/lapic.c | 2 ++
> > > > >  arch/x86/kvm/x86.c   | 7 +++++--
> > > > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > You also forgot to say what changed down here.
> > > > 
> > > > Don't know how strict the KVM maintainers are, I know I require these
> > > > things fixed up...
> > > 
> > > I require the same things, but I also don't mind doing fixup when applying if
> > > that's the path of least resistance (and it's not a recurring problem).
> > > 
> > > I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
> > > b4, and often confuses me as well.
> > > 
> > > But for this, I don't see any reason to send a v3.
> > 
> > Any status on this?  I don't see it in linux-next at all, nor in
> > 6.17-rc1
> 
> I'll get it applied and sent along to Paolo/Linus this week.

Thanks!

Re: [PATCH v2] KVM: x86: use array_index_nospec with indices that come from guest

[Sean Christopherson]

On Mon, Aug 11, 2025, Sean Christopherson wrote:
> On Mon, Aug 11, 2025, Greg Kroah-Hartman wrote:
> > On Thu, Jul 24, 2025 at 12:04:15PM -0700, Sean Christopherson wrote:
> > > On Thu, Jul 24, 2025, Greg Kroah-Hartman wrote:
> > > > On Thu, Jul 24, 2025 at 04:22:27PM +0200, Thijs Raymakers wrote:
> > > > > min and dest_id are guest-controlled indices. Using array_index_nospec()
> > > > > after the bounds checks clamps these values to mitigate speculative execution
> > > > > side-channels.
> > > > > 
> > > > > Signed-off-by: Thijs Raymakers <thijs@raymakers.nl>
> > > > > Cc: stable <stable@kernel.org>
> > > > > Cc: Sean Christopherson <seanjc@google.com>
> > > > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > 
> > > > Nit, you shouldn't have added my signed off on a new version, but that's
> > > > ok, I'm fine with it.
> > > 
> > > Want me to keep your SoB when applying, or drop it?
> > > 
> > > > > ---
> > > > >  arch/x86/kvm/lapic.c | 2 ++
> > > > >  arch/x86/kvm/x86.c   | 7 +++++--
> > > > >  2 files changed, 7 insertions(+), 2 deletions(-)
> > > > 
> > > > You also forgot to say what changed down here.
> > > > 
> > > > Don't know how strict the KVM maintainers are, I know I require these
> > > > things fixed up...
> > > 
> > > I require the same things, but I also don't mind doing fixup when applying if
> > > that's the path of least resistance (and it's not a recurring problem).
> > > 
> > > I also strongly dislike using In-Reply-To for new versions, as it tends to confuse
> > > b4, and often confuses me as well.
> > > 
> > > But for this, I don't see any reason to send a v3.
> > 
> > Any status on this?  I don't see it in linux-next at all, nor in
> > 6.17-rc1
> 
> I'll get it applied and sent along to Paolo/Linus this week.

I haven't forgotten about this, but I was out sick most of this week and v6.17-rc1
is crashing on my test systems, so I won't get this sent along until next week.
Sorry for the delay.