Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] irqchip/gic-v4: Harden against bogus command line
@ 2026-05-21 13:05 Mostafa Saleh
  2026-05-23  9:53 ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Mostafa Saleh @ 2026-05-21 13:05 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel; +Cc: maz, tglx, Mostafa Saleh

When accidentally setting “kvm-arm.vgic_v4_enable=1” on the wrong
setup that has no MSI controller device tree node (it exists but
not used) and GICv4, it caused a panic as “gic_domain” is NULL and
the kernel attempted to access its ops.

Originally, I hit this on an older kernel, but was able to reproduce
it on upstream with Qemu by hacking this unreasonable setup.

[   33.145536] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
[   33.145658] Mem abort info:
[   33.145751]   ESR = 0x0000000096000006
...
[   33.154057] CPU: 1 UID: 0 PID: 295 Comm: lkvm-static Not tainted 7.1.0-rc4-ge3f15ad3970e #5 PREEMPT
[   33.156922] Hardware name: linux,dummy-virt (DT)
[   33.158780] pstate: 81402005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[   33.160340] pc : __irq_domain_instantiate+0x1d4/0x578
[   33.162602] lr : __irq_domain_instantiate+0x1cc/0x578

Add a hardening check to avoid the NULL access, and fail the VM
creation in that case.

Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 drivers/irqchip/irq-gic-v4.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
index 8455b4a5fbb0..7e39f7eae85f 100644
--- a/drivers/irqchip/irq-gic-v4.c
+++ b/drivers/irqchip/irq-gic-v4.c
@@ -159,6 +159,9 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
 {
 	int vpe_base_irq, i;
 
+	if (!gic_domain)
+		return -EINVAL;
+
 	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv4-vpe",
 						      task_pid_nr(current));
 	if (!vm->fwnode)
-- 
2.54.0.669.g59709faab0-goog



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] irqchip/gic-v4: Harden against bogus command line
  2026-05-21 13:05 [PATCH] irqchip/gic-v4: Harden against bogus command line Mostafa Saleh
@ 2026-05-23  9:53 ` Marc Zyngier
  2026-05-26  7:50   ` Mostafa Saleh
  0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2026-05-23  9:53 UTC (permalink / raw)
  To: Mostafa Saleh; +Cc: linux-arm-kernel, linux-kernel, tglx

On Thu, 21 May 2026 14:05:03 +0100,
Mostafa Saleh <smostafa@google.com> wrote:
> 
> When accidentally setting “kvm-arm.vgic_v4_enable=1” on the wrong
> setup that has no MSI controller device tree node (it exists but
> not used) and GICv4, it caused a panic as “gic_domain” is NULL and
> the kernel attempted to access its ops.

When you say "that has no MSI controller device tree node", does it
mean that the ITS has not been probed at all?

>
> Originally, I hit this on an older kernel, but was able to reproduce
> it on upstream with Qemu by hacking this unreasonable setup.
> 
> [   33.145536] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
> [   33.145658] Mem abort info:
> [   33.145751]   ESR = 0x0000000096000006
> ...
> [   33.154057] CPU: 1 UID: 0 PID: 295 Comm: lkvm-static Not tainted 7.1.0-rc4-ge3f15ad3970e #5 PREEMPT
> [   33.156922] Hardware name: linux,dummy-virt (DT)
> [   33.158780] pstate: 81402005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> [   33.160340] pc : __irq_domain_instantiate+0x1d4/0x578
> [   33.162602] lr : __irq_domain_instantiate+0x1cc/0x578
> 
> Add a hardening check to avoid the NULL access, and fail the VM
> creation in that case.
> 
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>  drivers/irqchip/irq-gic-v4.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> index 8455b4a5fbb0..7e39f7eae85f 100644
> --- a/drivers/irqchip/irq-gic-v4.c
> +++ b/drivers/irqchip/irq-gic-v4.c
> @@ -159,6 +159,9 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
>  {
>  	int vpe_base_irq, i;
>  
> +	if (!gic_domain)
> +		return -EINVAL;
> +
>  	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv4-vpe",
>  						      task_pid_nr(current));
>  	if (!vm->fwnode)

I think this check is a good few levels too late. If you want to fix
this, I'd rather make sure that kvm_vgic_global_state.has_gicv4 is
reliable and covers this case. Which means making sure that
gic_kvm_info::has_v4 is itself reliable.

If my above understanding is correct, I'd expect the following
(untested) hack to help.

Thanks,

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index 291d7668cc8da..e6b9fee1b6786 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5838,6 +5838,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
 
 	if (list_empty(&its_nodes)) {
 		pr_warn("ITS: No ITS available, not enabling LPIs\n");
+		rdists->has_vlpis = false;
 		return -ENXIO;
 	}
 

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] irqchip/gic-v4: Harden against bogus command line
  2026-05-23  9:53 ` Marc Zyngier
@ 2026-05-26  7:50   ` Mostafa Saleh
  2026-05-26  9:45     ` Marc Zyngier
  0 siblings, 1 reply; 4+ messages in thread
From: Mostafa Saleh @ 2026-05-26  7:50 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: linux-arm-kernel, linux-kernel, tglx

On Sat, May 23, 2026 at 10:53:23AM +0100, Marc Zyngier wrote:
> On Thu, 21 May 2026 14:05:03 +0100,
> Mostafa Saleh <smostafa@google.com> wrote:
> > 
> > When accidentally setting “kvm-arm.vgic_v4_enable=1” on the wrong
> > setup that has no MSI controller device tree node (it exists but
> > not used) and GICv4, it caused a panic as “gic_domain” is NULL and
> > the kernel attempted to access its ops.
> 
> When you say "that has no MSI controller device tree node", does it
> mean that the ITS has not been probed at all?

Yes.

> 
> >
> > Originally, I hit this on an older kernel, but was able to reproduce
> > it on upstream with Qemu by hacking this unreasonable setup.
> > 
> > [   33.145536] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
> > [   33.145658] Mem abort info:
> > [   33.145751]   ESR = 0x0000000096000006
> > ...
> > [   33.154057] CPU: 1 UID: 0 PID: 295 Comm: lkvm-static Not tainted 7.1.0-rc4-ge3f15ad3970e #5 PREEMPT
> > [   33.156922] Hardware name: linux,dummy-virt (DT)
> > [   33.158780] pstate: 81402005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > [   33.160340] pc : __irq_domain_instantiate+0x1d4/0x578
> > [   33.162602] lr : __irq_domain_instantiate+0x1cc/0x578
> > 
> > Add a hardening check to avoid the NULL access, and fail the VM
> > creation in that case.
> > 
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> >  drivers/irqchip/irq-gic-v4.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> > index 8455b4a5fbb0..7e39f7eae85f 100644
> > --- a/drivers/irqchip/irq-gic-v4.c
> > +++ b/drivers/irqchip/irq-gic-v4.c
> > @@ -159,6 +159,9 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
> >  {
> >  	int vpe_base_irq, i;
> >  
> > +	if (!gic_domain)
> > +		return -EINVAL;
> > +
> >  	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv4-vpe",
> >  						      task_pid_nr(current));
> >  	if (!vm->fwnode)
> 
> I think this check is a good few levels too late. If you want to fix
> this, I'd rather make sure that kvm_vgic_global_state.has_gicv4 is
> reliable and covers this case. Which means making sure that
> gic_kvm_info::has_v4 is itself reliable.
> 
> If my above understanding is correct, I'd expect the following
> (untested) hack to help.

Thanks! That also fixes the crash, the VM will launch with a vGIC with
no ITS in that case.

Thanks,
Mostafa

> 
> Thanks,
> 
> 	M.
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index 291d7668cc8da..e6b9fee1b6786 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -5838,6 +5838,7 @@ int __init its_init(struct fwnode_handle *handle, struct rdists *rdists,
>  
>  	if (list_empty(&its_nodes)) {
>  		pr_warn("ITS: No ITS available, not enabling LPIs\n");
> +		rdists->has_vlpis = false;
>  		return -ENXIO;
>  	}
>  
> 
> -- 
> Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] irqchip/gic-v4: Harden against bogus command line
  2026-05-26  7:50   ` Mostafa Saleh
@ 2026-05-26  9:45     ` Marc Zyngier
  0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2026-05-26  9:45 UTC (permalink / raw)
  To: Mostafa Saleh; +Cc: linux-arm-kernel, linux-kernel, tglx

On Tue, 26 May 2026 08:50:13 +0100,
Mostafa Saleh <smostafa@google.com> wrote:
> 
> On Sat, May 23, 2026 at 10:53:23AM +0100, Marc Zyngier wrote:
> > On Thu, 21 May 2026 14:05:03 +0100,
> > Mostafa Saleh <smostafa@google.com> wrote:
> > > 
> > > When accidentally setting “kvm-arm.vgic_v4_enable=1” on the wrong
> > > setup that has no MSI controller device tree node (it exists but
> > > not used) and GICv4, it caused a panic as “gic_domain” is NULL and
> > > the kernel attempted to access its ops.
> > 
> > When you say "that has no MSI controller device tree node", does it
> > mean that the ITS has not been probed at all?
> 
> Yes.
> 
> > 
> > >
> > > Originally, I hit this on an older kernel, but was able to reproduce
> > > it on upstream with Qemu by hacking this unreasonable setup.
> > > 
> > > [   33.145536] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
> > > [   33.145658] Mem abort info:
> > > [   33.145751]   ESR = 0x0000000096000006
> > > ...
> > > [   33.154057] CPU: 1 UID: 0 PID: 295 Comm: lkvm-static Not tainted 7.1.0-rc4-ge3f15ad3970e #5 PREEMPT
> > > [   33.156922] Hardware name: linux,dummy-virt (DT)
> > > [   33.158780] pstate: 81402005 (Nzcv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
> > > [   33.160340] pc : __irq_domain_instantiate+0x1d4/0x578
> > > [   33.162602] lr : __irq_domain_instantiate+0x1cc/0x578
> > > 
> > > Add a hardening check to avoid the NULL access, and fail the VM
> > > creation in that case.
> > > 
> > > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > > ---
> > >  drivers/irqchip/irq-gic-v4.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> > > index 8455b4a5fbb0..7e39f7eae85f 100644
> > > --- a/drivers/irqchip/irq-gic-v4.c
> > > +++ b/drivers/irqchip/irq-gic-v4.c
> > > @@ -159,6 +159,9 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
> > >  {
> > >  	int vpe_base_irq, i;
> > >  
> > > +	if (!gic_domain)
> > > +		return -EINVAL;
> > > +
> > >  	vm->fwnode = irq_domain_alloc_named_id_fwnode("GICv4-vpe",
> > >  						      task_pid_nr(current));
> > >  	if (!vm->fwnode)
> > 
> > I think this check is a good few levels too late. If you want to fix
> > this, I'd rather make sure that kvm_vgic_global_state.has_gicv4 is
> > reliable and covers this case. Which means making sure that
> > gic_kvm_info::has_v4 is itself reliable.
> > 
> > If my above understanding is correct, I'd expect the following
> > (untested) hack to help.
> 
> Thanks! That also fixes the crash, the VM will launch with a vGIC with
> no ITS in that case.

You should still be able to have a virtual ITS, just no direct
injection of any sort.

If you're happy with that, please respin a patch with this hack.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-26  9:45 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21 13:05 [PATCH] irqchip/gic-v4: Harden against bogus command line Mostafa Saleh
2026-05-23  9:53 ` Marc Zyngier
2026-05-26  7:50   ` Mostafa Saleh
2026-05-26  9:45     ` Marc Zyngier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox