In certain cases intel_th_irq can reference a null entry in
the th->thdev array.  This results in the splat shown below.
The problem is that intel_th_output_enable() can modify the
thdev[] array at the same time intel_th_irq is referencing
the same array.  This can be fixed by disabling interrupts
during the call to intel_th_output_enable().

BUG: kernel NULL pointer dereference, address: 0000000000000304
Oops: Oops: 0000 [#1] SMP NOPTI
RIP: 0010:intel_th_irq+0x26/0x70 [intel_th]
Call Trace:
 <IRQ>
 ? show_trace_log_lvl+0x1b0/0x2f0
 ? show_trace_log_lvl+0x1b0/0x2f0
 ? __handle_irq_event_percpu+0x4a/0x180
 ? __die_body.cold+0x8/0x12
 ? page_fault_oops+0x148/0x160
 ? exc_page_fault+0x73/0x160
 ? asm_exc_page_fault+0x26/0x30
 ? intel_th_irq+0x26/0x70 [intel_th]
 __handle_irq_event_percpu+0x4a/0x180
 handle_irq_event+0x38/0x80
handle_fasteoi_irq+0x78/0x200
__common_interrupt+0x3e/0x90
common_interrupt+0x80/0xa0
</IRQ>

Fixes: a753bfcfdb1f ("intel_th: Make the switch allocate its subdevices")
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: linux-kernel@vger.kernel.org
Cc: Jerry Hoemann <jerry.hoemann@hpe.com>
Signed-off-by: David Arcari <darcari@redhat.com>
---
v2: switched to use scoped based lock guard functionality

drivers/hwtracing/intel_th/core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hwtracing/intel_th/core.c b/drivers/hwtracing/intel_th/core.c
index 47d9e6c3bac0..7b7dcc651efe 100644
--- a/drivers/hwtracing/intel_th/core.c
+++ b/drivers/hwtracing/intel_th/core.c
@@ -717,6 +717,8 @@ int intel_th_output_enable(struct intel_th *th, unsigned int otype)
 	struct intel_th_device *thdev;
 	int src = 0, dst = 0;
 
+	guard(disable_irq)(&th->irq);
+
 	for (src = 0, dst = 0; dst <= th->num_thdevs; src++, dst++) {
 		for (; src < ARRAY_SIZE(intel_th_subdevices); src++) {
 			if (intel_th_subdevices[src].type != INTEL_TH_OUTPUT)
-- 
2.51.0

> In certain cases intel_th_irq can reference a null entry in
> the th->thdev array.  This results in the splat shown below.
> The problem is that intel_th_output_enable() can modify the
> thdev[] array at the same time intel_th_irq is referencing
> the same array.  This can be fixed by disabling interrupts
> during the call to intel_th_output_enable().

1. Would another imperative wording become helpful for an improved change description?
   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc6#n94

2. You may occasionally put more than 60 characters into text lines
   of such a change description.

3. Would a summary phrase like “Prevent null pointer dereference
   in intel_th_output_enable()” be more appropriate?


Regards,
Markus

Hi,

On 11/19/25 7:55 AM, Markus Elfring wrote:
>> In certain cases intel_th_irq can reference a null entry in
>> the th->thdev array.  This results in the splat shown below.
>> The problem is that intel_th_output_enable() can modify the
>> thdev[] array at the same time intel_th_irq is referencing
>> the same array.  This can be fixed by disabling interrupts
>> during the call to intel_th_output_enable().
> 
> 1. Would another imperative wording become helpful for an improved change description?
>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc6#n94

I feel like the description explains the problem well. However, do you 
have alternate wording you would like to suggest?

> 
> 2. You may occasionally put more than 60 characters into text lines
>     of such a change description.
I can redo the body of the commit and submit a v3 if the maintainer is 
interested in applying a patch of this nature.

> 
> 3. Would a summary phrase like “Prevent null pointer dereference
>     in intel_th_output_enable()” be more appropriate?
The null pointer deference occurs in intel_th_irq. So I could change it 
to "Prevent null pointer dererference in intel_th_irq".

Before I do anything else with this patch I'd like to hear back from 
Alexander. There's no reason to refactor a patch that won't be committed.

Thanks,
-DA

> 
> 
> Regards,
> Markus
> 

…>>> the same array.  This can be fixed by disabling interrupts
>>> during the call to intel_th_output_enable().
>>
>> 1. Would another imperative wording become helpful for an improved change description?
>>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc6#n94
> 
> I feel like the description explains the problem well.

Perhaps?


> However, do you have alternate wording you would like to suggest?

Under which circumstances would you get into the mood to convert the wording approach
“can be fixed by” into an imperative one?

Regards,
Markus

On 11/20/25 8:07 AM, Markus Elfring wrote:
> …>>> the same array.  This can be fixed by disabling interrupts
>>>> during the call to intel_th_output_enable().
>>>
>>> 1. Would another imperative wording become helpful for an improved change description?
>>>      https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc6#n94
>>
>> I feel like the description explains the problem well.
> 
> Perhaps?
> 
> 
>> However, do you have alternate wording you would like to suggest?
> 
> Under which circumstances would you get into the mood to convert the wording approach
> “can be fixed by” into an imperative one?

I see. I will make that change if a v3 is in order. Naturally, I'd like 
to hear back from Alexander before putting more effort into this.

Best,
-DA

> 
> Regards,
> Markus
>