[PATCH] genirq/msi: Fix MSI domain debugfs show

[PATCH] genirq/msi: Fix MSI domain debugfs show

[Andrew Jones]

irq_domain_debug_show_one() calls the irqdomain's debug_show() with
a non-null domain pointer and a null irqdata pointer
(irq_debug_show_data() calls debug_show() with those the other way
around). Ensure we have a non-null irqdata pointer in
msi_domain_debug_show() before dereferencing it.

Fixes: 01499ae673dc ("genirq/msi: Expose MSI message data in debugfs")
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 kernel/irq/msi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 5c8d43cdb0a3..c05ba7ca00fa 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -761,7 +761,7 @@ static int msi_domain_translate(struct irq_domain *domain, struct irq_fwspec *fw
 static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d,
 				  struct irq_data *irqd, int ind)
 {
-	struct msi_desc *desc = irq_data_get_msi_desc(irqd);
+	struct msi_desc *desc = irqd ? irq_data_get_msi_desc(irqd) : NULL;
 
 	if (!desc)
 		return;
-- 
2.49.0

[tip: irq/urgent] genirq/msi: Prevent NULL pointer dereference in msi_domain_debug_show()

[tip-bot2 for Andrew Jones]

The following commit has been merged into the irq/urgent branch of tip:

Commit-ID:     e6a3fc4f10b872d02e25f83227e725c79b25d893
Gitweb:        https://git.kernel.org/tip/e6a3fc4f10b872d02e25f83227e725c79b25d893
Author:        Andrew Jones <ajones@ventanamicro.com>
AuthorDate:    Wed, 30 Apr 2025 14:48:37 +02:00
Committer:     Thomas Gleixner <tglx@linutronix.de>
CommitterDate: Wed, 30 Apr 2025 23:25:10 +02:00

genirq/msi: Prevent NULL pointer dereference in msi_domain_debug_show()

irq_domain_debug_show_one() calls msi_domain_debug_show() with a non-NULL
domain pointer and a NULL irq_data pointer. irq_debug_show_data() calls it
with a NULL domain pointer.

The domain pointer is not used, but the irq_data pointer is required to be
non-NULL and lacks a NULL pointer check.

Add the missing NULL pointer check to ensure there is a non-NULL irq_data
pointer in msi_domain_debug_show() before dereferencing it.

[ tglx: Massaged change log ]

Fixes: 01499ae673dc ("genirq/msi: Expose MSI message data in debugfs")
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Link: https://lore.kernel.org/all/20250430124836.49964-2-ajones@ventanamicro.com
---
 kernel/irq/msi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index 5c8d43c..c05ba7c 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -761,7 +761,7 @@ static int msi_domain_translate(struct irq_domain *domain, struct irq_fwspec *fw
 static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d,
 				  struct irq_data *irqd, int ind)
 {
-	struct msi_desc *desc = irq_data_get_msi_desc(irqd);
+	struct msi_desc *desc = irqd ? irq_data_get_msi_desc(irqd) : NULL;
 
 	if (!desc)
 		return;

Re: [PATCH] genirq/msi: Fix MSI domain debugfs show

[Hans Zhang]

On 2025/4/30 20:48, Andrew Jones wrote:
> irq_domain_debug_show_one() calls the irqdomain's debug_show() with
> a non-null domain pointer and a null irqdata pointer
> (irq_debug_show_data() calls debug_show() with those the other way
> around). Ensure we have a non-null irqdata pointer in
> msi_domain_debug_show() before dereferencing it.
> 
> Fixes: 01499ae673dc ("genirq/msi: Expose MSI message data in debugfs")
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>   kernel/irq/msi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index 5c8d43cdb0a3..c05ba7ca00fa 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -761,7 +761,7 @@ static int msi_domain_translate(struct irq_domain *domain, struct irq_fwspec *fw
>   static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d,
>   				  struct irq_data *irqd, int ind)
>   {
> -	struct msi_desc *desc = irq_data_get_msi_desc(irqd);
> +	struct msi_desc *desc = irqd ? irq_data_get_msi_desc(irqd) : NULL;
>   

Hi Andrew,

I think irqd will never be NULL.


static int irq_debug_show(struct seq_file *m, void *p)
   irq_debug_show_data(m, data, 0);
     if (data->domain && data->domain->ops && data->domain->ops->debug_show)
       data->domain->ops->debug_show(m, NULL, data, ind + 1);
         msi_domain_debug_show



static int irq_debug_show(struct seq_file *m, void *p)
{
	struct irq_desc *desc = m->private;
	struct irq_data *data;

	raw_spin_lock_irq(&desc->lock);
	data = irq_desc_get_irq_data(desc);

	If it needs to be judged as NULL, should it also be here?



static void irq_domain_debug_show_one(struct seq_file *m, struct 
irq_domain *d, int ind)
   if (d->ops && d->ops->debug_show)	// d->ops->debug_show is NULL, not 
irq_debug_show.


If I'm wrong, please correct me.

Best regards,
Hans

>   	if (!desc)
>   		return;

Re: [PATCH] genirq/msi: Fix MSI domain debugfs show

[Andrew Jones]

On Thu, May 01, 2025 at 09:51:07AM +0800, Hans Zhang wrote:
> 
> 
> On 2025/4/30 20:48, Andrew Jones wrote:
> > irq_domain_debug_show_one() calls the irqdomain's debug_show() with
> > a non-null domain pointer and a null irqdata pointer
> > (irq_debug_show_data() calls debug_show() with those the other way
> > around). Ensure we have a non-null irqdata pointer in
> > msi_domain_debug_show() before dereferencing it.
> > 
> > Fixes: 01499ae673dc ("genirq/msi: Expose MSI message data in debugfs")
> > Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> >   kernel/irq/msi.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> > index 5c8d43cdb0a3..c05ba7ca00fa 100644
> > --- a/kernel/irq/msi.c
> > +++ b/kernel/irq/msi.c
> > @@ -761,7 +761,7 @@ static int msi_domain_translate(struct irq_domain *domain, struct irq_fwspec *fw
> >   static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d,
> >   				  struct irq_data *irqd, int ind)
> >   {
> > -	struct msi_desc *desc = irq_data_get_msi_desc(irqd);
> > +	struct msi_desc *desc = irqd ? irq_data_get_msi_desc(irqd) : NULL;
> 
> Hi Andrew,
> 
> I think irqd will never be NULL.
> 
> 
> static int irq_debug_show(struct seq_file *m, void *p)
>   irq_debug_show_data(m, data, 0);
>     if (data->domain && data->domain->ops && data->domain->ops->debug_show)
>       data->domain->ops->debug_show(m, NULL, data, ind + 1);
>         msi_domain_debug_show
> 
> 
> 
> static int irq_debug_show(struct seq_file *m, void *p)
> {
> 	struct irq_desc *desc = m->private;
> 	struct irq_data *data;
> 
> 	raw_spin_lock_irq(&desc->lock);
> 	data = irq_desc_get_irq_data(desc);
> 
> 	If it needs to be judged as NULL, should it also be here?
> 
> 
> 
> static void irq_domain_debug_show_one(struct seq_file *m, struct irq_domain
> *d, int ind)
>   if (d->ops && d->ops->debug_show)	// d->ops->debug_show is NULL, not
> irq_debug_show.
> 
> 
> If I'm wrong, please correct me.
>

# cat /sys/kernel/debug/irq/domains/PCI-MSIX-0000:00:01.0-12 
[   51.954695] Unable to handle kernel access to user memory without uaccess routines at virtual address 0000000000000010
[   51.956868] Current cat pgtable: 4K pagesize, 57-bit VAs, pgdp=0x0000000101a9a000
[   51.957207] [0000000000000010] pgd=00000000407e8801, p4d=00000000406a9001, pud=00000000407ac001, pmd=00000000407ad001, pte=0000000000000000
[   51.958324] Oops [#1]
[   51.958417] Modules linked in:
[   51.958721] CPU: 1 UID: 0 PID: 83 Comm: cat Not tainted 6.15.0-rc4-00147-gebd297a2affa #1 NONE 
[   51.958993] Hardware name: riscv-virtio,qemu (DT)
[   51.959196] epc : msi_domain_debug_show+0xa/0x8a
[   51.959736]  ra : irq_domain_debug_show_one+0xb6/0xf4
[   51.959871] epc : ffffffff800a26a4 ra : ffffffff8009f064 sp : ff2000000035bc10
[   51.960035]  gp : ffffffff81516710 tp : ff600000802d7080 t0 : ff600000809f20c3
[   51.960204]  t1 : 00000000000000de t2 : ffffffff80e01470 s0 : ff2000000035bc40
[   51.960373]  s1 : ff60000080285f00 a0 : ff60000081eb5000 a1 : ff60000080285f00
[   51.960533]  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
[   51.960697]  a5 : ffffffff800a269a a6 : 0000000000000000 a7 : 0000000000000010
[   51.960854]  s2 : ff60000081eb5000 s3 : 0000000000000000 s4 : 0000000000000001
[   51.961017]  s5 : ff2000000035bd00 s6 : 0000000000000000 s7 : ff60000081eb5028
[   51.961188]  s8 : fffffffffffff000 s9 : ff60000081eb5038 s10: 000000007ffff000
[   51.961349]  s11: 0000000000000000 t3 : ffffffff80e5d850 t4 : 0000000000174000
[   51.961509]  t5 : 0000000000000002 t6 : ff600000809f20de
[   51.961635] status: 0000000200000120 badaddr: 0000000000000010 cause: 000000000000000d
[   51.961886] [<ffffffff800a26a4>] msi_domain_debug_show+0xa/0x8a
[   51.962098] [<ffffffff8009f064>] irq_domain_debug_show_one+0xb6/0xf4
[   51.962250] [<ffffffff8009f0b4>] irq_domain_debug_show+0x12/0x2a
[   51.962389] [<ffffffff80246bd6>] seq_read_iter+0xc6/0x316
[   51.962516] [<ffffffff80246f12>] seq_read+0xec/0x11e
[   51.962634] [<ffffffff80381d20>] full_proxy_read+0x48/0x88
[   51.962771] [<ffffffff802184de>] vfs_read+0xb2/0x288
[   51.962897] [<ffffffff80218ecc>] ksys_read+0x56/0xc0
[   51.963012] [<ffffffff80218f4a>] __riscv_sys_read+0x14/0x1c
[   51.963147] [<ffffffff809f3566>] do_trap_ecall_u+0x186/0x206
[   51.963290] [<ffffffff809fdf42>] handle_exception+0x146/0x152
[   51.963588] Code: 0209 9181 b7a5 4981 b759 7179 f022 f406 e44e 1800 (6a1c) b983 
[   51.963910] ---[ end trace 0000000000000000 ]---
Segmentation fault

Re: [PATCH] genirq/msi: Fix MSI domain debugfs show

[Hans Zhang]

On 2025/5/2 15:33, Andrew Jones wrote:
> On Thu, May 01, 2025 at 09:51:07AM +0800, Hans Zhang wrote:
>>
>>
>> On 2025/4/30 20:48, Andrew Jones wrote:
>>> irq_domain_debug_show_one() calls the irqdomain's debug_show() with
>>> a non-null domain pointer and a null irqdata pointer
>>> (irq_debug_show_data() calls debug_show() with those the other way
>>> around). Ensure we have a non-null irqdata pointer in
>>> msi_domain_debug_show() before dereferencing it.
>>>
>>> Fixes: 01499ae673dc ("genirq/msi: Expose MSI message data in debugfs")
>>> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
>>> ---
>>>    kernel/irq/msi.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
>>> index 5c8d43cdb0a3..c05ba7ca00fa 100644
>>> --- a/kernel/irq/msi.c
>>> +++ b/kernel/irq/msi.c
>>> @@ -761,7 +761,7 @@ static int msi_domain_translate(struct irq_domain *domain, struct irq_fwspec *fw
>>>    static void msi_domain_debug_show(struct seq_file *m, struct irq_domain *d,
>>>    				  struct irq_data *irqd, int ind)
>>>    {
>>> -	struct msi_desc *desc = irq_data_get_msi_desc(irqd);
>>> +	struct msi_desc *desc = irqd ? irq_data_get_msi_desc(irqd) : NULL;
>>
>> Hi Andrew,
>>
>> I think irqd will never be NULL.
>>
>>
>> static int irq_debug_show(struct seq_file *m, void *p)
>>    irq_debug_show_data(m, data, 0);
>>      if (data->domain && data->domain->ops && data->domain->ops->debug_show)
>>        data->domain->ops->debug_show(m, NULL, data, ind + 1);
>>          msi_domain_debug_show
>>
>>
>>
>> static int irq_debug_show(struct seq_file *m, void *p)
>> {
>> 	struct irq_desc *desc = m->private;
>> 	struct irq_data *data;
>>
>> 	raw_spin_lock_irq(&desc->lock);
>> 	data = irq_desc_get_irq_data(desc);
>>
>> 	If it needs to be judged as NULL, should it also be here?
>>
>>
>>
>> static void irq_domain_debug_show_one(struct seq_file *m, struct irq_domain
>> *d, int ind)
>>    if (d->ops && d->ops->debug_show)	// d->ops->debug_show is NULL, not
>> irq_debug_show.
>>
>>
>> If I'm wrong, please correct me.
>>
> 
> # cat /sys/kernel/debug/irq/domains/PCI-MSIX-0000:00:01.0-12
> [   51.954695] Unable to handle kernel access to user memory without uaccess routines at virtual address 0000000000000010
> [   51.956868] Current cat pgtable: 4K pagesize, 57-bit VAs, pgdp=0x0000000101a9a000
> [   51.957207] [0000000000000010] pgd=00000000407e8801, p4d=00000000406a9001, pud=00000000407ac001, pmd=00000000407ad001, pte=0000000000000000
> [   51.958324] Oops [#1]
> [   51.958417] Modules linked in:
> [   51.958721] CPU: 1 UID: 0 PID: 83 Comm: cat Not tainted 6.15.0-rc4-00147-gebd297a2affa #1 NONE
> [   51.958993] Hardware name: riscv-virtio,qemu (DT)
> [   51.959196] epc : msi_domain_debug_show+0xa/0x8a
> [   51.959736]  ra : irq_domain_debug_show_one+0xb6/0xf4
> [   51.959871] epc : ffffffff800a26a4 ra : ffffffff8009f064 sp : ff2000000035bc10
> [   51.960035]  gp : ffffffff81516710 tp : ff600000802d7080 t0 : ff600000809f20c3
> [   51.960204]  t1 : 00000000000000de t2 : ffffffff80e01470 s0 : ff2000000035bc40
> [   51.960373]  s1 : ff60000080285f00 a0 : ff60000081eb5000 a1 : ff60000080285f00
> [   51.960533]  a2 : 0000000000000000 a3 : 0000000000000001 a4 : 0000000000000000
> [   51.960697]  a5 : ffffffff800a269a a6 : 0000000000000000 a7 : 0000000000000010
> [   51.960854]  s2 : ff60000081eb5000 s3 : 0000000000000000 s4 : 0000000000000001
> [   51.961017]  s5 : ff2000000035bd00 s6 : 0000000000000000 s7 : ff60000081eb5028
> [   51.961188]  s8 : fffffffffffff000 s9 : ff60000081eb5038 s10: 000000007ffff000
> [   51.961349]  s11: 0000000000000000 t3 : ffffffff80e5d850 t4 : 0000000000174000
> [   51.961509]  t5 : 0000000000000002 t6 : ff600000809f20de
> [   51.961635] status: 0000000200000120 badaddr: 0000000000000010 cause: 000000000000000d
> [   51.961886] [<ffffffff800a26a4>] msi_domain_debug_show+0xa/0x8a
> [   51.962098] [<ffffffff8009f064>] irq_domain_debug_show_one+0xb6/0xf4
> [   51.962250] [<ffffffff8009f0b4>] irq_domain_debug_show+0x12/0x2a
> [   51.962389] [<ffffffff80246bd6>] seq_read_iter+0xc6/0x316
> [   51.962516] [<ffffffff80246f12>] seq_read+0xec/0x11e
> [   51.962634] [<ffffffff80381d20>] full_proxy_read+0x48/0x88
> [   51.962771] [<ffffffff802184de>] vfs_read+0xb2/0x288
> [   51.962897] [<ffffffff80218ecc>] ksys_read+0x56/0xc0
> [   51.963012] [<ffffffff80218f4a>] __riscv_sys_read+0x14/0x1c
> [   51.963147] [<ffffffff809f3566>] do_trap_ecall_u+0x186/0x206
> [   51.963290] [<ffffffff809fdf42>] handle_exception+0x146/0x152
> [   51.963588] Code: 0209 9181 b7a5 4981 b759 7179 f022 f406 e44e 1800 (6a1c) b983
> [   51.963910] ---[ end trace 0000000000000000 ]---
> Segmentation fault


Ok, I see. Thank you for fixing this problem.

Reviewed-by: Hans Zhang <18255117159@163.com>

Best regards,
Hans