mbox series

[v2,0/2] KVM: arm64: Assorted vgic fixes for 6.14

Message ID 20250212182558.2865232-1-maz@kernel.org (mailing list archive)
Headers show
Series KVM: arm64: Assorted vgic fixes for 6.14 | expand

Message

Marc Zyngier Feb. 12, 2025, 6:25 p.m. UTC
Alexander, while fuzzing KVM/arm64, found an annoying set of problems,
all stemming from the fact that the vgic can be initialised or
destroyed in parallel with the rest of the guest still being live.

Yes, this is annoying.

This second version takes a different approach at the problem,
plugging the glaring hole we have between vgic creation and private
interrupt allocation.

Although this is more invasive, I'm more confident about this one than
the initial version I posted a week ago.

Alex, I'd very much appreciate your testing on this.

Marc Zyngier (2):
  KVM: arm64: timer: Drop warning on failed interrupt signalling
  KVM: arm64: vgic: Hoist SGI/PPI alloc from vgic_init() to
    kvm_create_vgic()

 arch/arm64/kvm/arch_timer.c     | 16 ++++---
 arch/arm64/kvm/vgic/vgic-init.c | 74 ++++++++++++++++-----------------
 2 files changed, 44 insertions(+), 46 deletions(-)

Comments

Oliver Upton Feb. 13, 2025, 4:59 a.m. UTC | #1
On Wed, Feb 12, 2025 at 06:25:56PM +0000, Marc Zyngier wrote:
> Alexander, while fuzzing KVM/arm64, found an annoying set of problems,
> all stemming from the fact that the vgic can be initialised or
> destroyed in parallel with the rest of the guest still being live.
> 
> Yes, this is annoying.
> 
> This second version takes a different approach at the problem,
> plugging the glaring hole we have between vgic creation and private
> interrupt allocation.
> 
> Although this is more invasive, I'm more confident about this one than
> the initial version I posted a week ago.

Much better place now! Here's to the next pile of syzkaller bugs :)

Reviewed-by: Oliver Upton <oliver.upton@linux.dev>

> Alex, I'd very much appreciate your testing on this.

I too would like to see the tires kicked before we pick this up, if it
isn't too much trouble Alex.

> Marc Zyngier (2):
>   KVM: arm64: timer: Drop warning on failed interrupt signalling
>   KVM: arm64: vgic: Hoist SGI/PPI alloc from vgic_init() to
>     kvm_create_vgic()
> 
>  arch/arm64/kvm/arch_timer.c     | 16 ++++---
>  arch/arm64/kvm/vgic/vgic-init.c | 74 ++++++++++++++++-----------------
>  2 files changed, 44 insertions(+), 46 deletions(-)
> 
> -- 
> 2.39.2
>
Alexander Potapenko Feb. 13, 2025, 10:29 a.m. UTC | #2
On Thu, Feb 13, 2025 at 5:59 AM Oliver Upton <oliver.upton@linux.dev> wrote:
>
> On Wed, Feb 12, 2025 at 06:25:56PM +0000, Marc Zyngier wrote:
> > Alexander, while fuzzing KVM/arm64, found an annoying set of problems,
> > all stemming from the fact that the vgic can be initialised or
> > destroyed in parallel with the rest of the guest still being live.
> >
> > Yes, this is annoying.
> >
> > This second version takes a different approach at the problem,
> > plugging the glaring hole we have between vgic creation and private
> > interrupt allocation.
> >
> > Although this is more invasive, I'm more confident about this one than
> > the initial version I posted a week ago.
>
> Much better place now! Here's to the next pile of syzkaller bugs :)
>
> Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
>
> > Alex, I'd very much appreciate your testing on this.
>
> I too would like to see the tires kicked before we pick this up, if it
> isn't too much trouble Alex.

I am on it, will report back today or tomorrow.
Alexander Potapenko Feb. 14, 2025, 6:25 p.m. UTC | #3
On Thu, Feb 13, 2025 at 11:29 AM Alexander Potapenko <glider@google.com> wrote:
>
> On Thu, Feb 13, 2025 at 5:59 AM Oliver Upton <oliver.upton@linux.dev> wrote:
> >
> > On Wed, Feb 12, 2025 at 06:25:56PM +0000, Marc Zyngier wrote:
> > > Alexander, while fuzzing KVM/arm64, found an annoying set of problems,
> > > all stemming from the fact that the vgic can be initialised or
> > > destroyed in parallel with the rest of the guest still being live.
> > >
> > > Yes, this is annoying.
> > >
> > > This second version takes a different approach at the problem,
> > > plugging the glaring hole we have between vgic creation and private
> > > interrupt allocation.
> > >
> > > Although this is more invasive, I'm more confident about this one than
> > > the initial version I posted a week ago.
> >
> > Much better place now! Here's to the next pile of syzkaller bugs :)
> >
> > Reviewed-by: Oliver Upton <oliver.upton@linux.dev>
> >
> > > Alex, I'd very much appreciate your testing on this.
> >
> > I too would like to see the tires kicked before we pick this up, if it
> > isn't too much trouble Alex.
>
> I am on it, will report back today or tomorrow.

I am seeing the following crashes, do you think these could be related
to your changes?

==================================================================
BUG: KASAN: null-ptr-deref in _raw_spin_lock_irqsave+0xa8/0x174
include/linux/instrumented.h:96
Write of size 4 at addr 0000000000000d20 by task syz.3.8387/5166

CPU: 1 UID: 0 PID: 5166 Comm: syz.3.8387 Not tainted
6.14.0-rc2-00002-g4b305a8c5b85 #159
Hardware name: linux,dummy-virt (DT)
Call trace:
 show_stack+0x2c/0x3c arch/arm64/kernel/stacktrace.c:466 (C)
 __dump_stack lib/dump_stack.c:94 [inline]
 dump_stack_lvl+0x94/0xc0 lib/dump_stack.c:120
 print_report+0xf8/0x7d4 mm/kasan/report.c:492
 kasan_report+0xcc/0x128 mm/kasan/report.c:602
 kasan_check_range+0x264/0x2a4
 __kasan_check_write+0x20/0x30 mm/kasan/shadow.c:37
 _raw_spin_lock_irqsave+0xa8/0x174 include/linux/instrumented.h:96
 kvm_vgic_set_owner+0x15c/0x23c arch/arm64/kvm/vgic/vgic.c:611
 kvm_timer_enable+0x174/0x5b0 arch/arm64/kvm/arch_timer.c:1574
 kvm_arch_vcpu_run_pid_change+0x184/0x28c arch/arm64/kvm/arm.c:824
 kvm_vcpu_ioctl+0xa94/0xba8 virt/kvm/kvm_main.c:4366
 __do_sys_ioctl fs/ioctl.c:51 [inline]
...