linux-mediatek.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port
@ 2025-11-11 12:48 Jian Hui Lee
  2025-11-12 15:51 ` Krzysztof Kozlowski
  0 siblings, 1 reply; 5+ messages in thread
From: Jian Hui Lee @ 2025-11-11 12:48 UTC (permalink / raw)
  To: Yong Wu, Krzysztof Kozlowski, Matthias Brugger,
	AngeloGioacchino Del Regno, Joerg Roedel, linux-mediatek,
	linux-kernel, linux-arm-kernel

Fix a race condition where mtk_smi_larb_resume() can be called before
mtk_smi_larb_bind() completes, leading to NULL pointer dereference when
accessing larb->mmu and larb->bank.

Add NULL checks to skip port configuration gracefully when the device
hasn't been bound yet.

Crash trace:
[    7.605014] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[    7.605029] Mem abort info:
[    7.605031]   ESR = 0x0000000096000004
[    7.605034]   EC = 0x25: DABT (current EL), IL = 32 bits
[    7.605037]   SET = 0, FnV = 0
[    7.605040]   EA = 0, S1PTW = 0
[    7.605042]   FSC = 0x04: level 0 translation fault
[    7.605045] Data abort info:
[    7.605046]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[    7.605049]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[    7.605052]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[    7.605055] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000106755000
[    7.605059] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
[    7.605069] Internal error: Oops: 0000000096000004 [#1]  SMP
[    7.605075] Modules linked in: mtk_mdp3(+) snd_compress(+) ac97_bus snd_pcm_dmaengine snd_pcm v4l2_mem2mem videobufo
[    7.605223]  pwm_mtk_disp spmi_mtk_pmif spmi_devres clk_mt8188_imp_iic_wrap spi_mt65xx mmc_hsq stmmac clk_mt8188_ado
[    7.605267] CPU: 5 UID: 0 PID: 417 Comm: (udev-worker) Not tainted 6.17.2-custom #29 PREEMPT(voluntary)
[    7.605273] Hardware name: mediatek Genio 700 EVK P1V4 (eMMC Boot)/Genio 700 EVK P1V4, BIOS 2022.10-gb56d2a20f5 10/2
[    7.605278] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    7.605283] pc : mtk_smi_larb_config_port_gen2_general+0x2a8/0x458 [mtk_smi]
[    7.605301] lr : mtk_smi_larb_config_port_gen2_general+0x110/0x458 [mtk_smi]
[    7.605307] sp : ffff8000867f3370
[    7.605309] x29: ffff8000867f33e0 x28: 0000000000000000 x27: ffff80007c403fcc
[    7.605317] x26: 000000000000000a x25: ffff0000d5107c80 x24: ffff80007c4040f8
[    7.605325] x23: ffff80007c4040f8 x22: ffff80007c403fcc x21: ffff80007c43c918
[    7.605332] x20: 0000000000000007 x19: ffff80008685d218 x18: ffff8000863b90e8
[    7.605340] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[    7.605346] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[    7.605353] x11: 0000000000000000 x10: 0000000000000000 x9 : ffff80007c403fcc
[    7.605360] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
[    7.605367] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
[    7.605373] x2 : 0000000000000000 x1 : ffff8000867f33b8 x0 : 0000000000000000
[    7.605381] Call trace:
[    7.605384]  mtk_smi_larb_config_port_gen2_general+0x2a8/0x458 [mtk_smi] (P)
[    7.605393]  mtk_smi_larb_resume+0xbc/0x130 [mtk_smi]
[    7.605399]  pm_generic_runtime_resume+0x38/0x80
[    7.605410]  __genpd_runtime_resume+0x3c/0xb8
[    7.605419]  genpd_runtime_resume+0xec/0x338
[    7.605425]  __rpm_callback+0x54/0x210
[    7.605431]  rpm_callback+0x94/0xa8
[    7.605436]  rpm_resume+0x484/0x668
[    7.605442]  __pm_runtime_resume+0x68/0xd8
[    7.605447]  pm_runtime_get_suppliers+0x6c/0xb8
[    7.605453]  __driver_probe_device+0x5c/0x1c8
[    7.605461]  driver_probe_device+0x48/0x188
[    7.605466]  __driver_attach+0x14c/0x2c8
[    7.605471]  bus_for_each_dev+0x88/0x110
[    7.605476]  driver_attach+0x30/0x60
[    7.605481]  bus_add_driver+0x17c/0x2e8
[    7.605486]  driver_register+0x68/0x178
[    7.605492]  __platform_driver_register+0x30/0x60
[    7.605498]  mdp_driver_init+0x2c/0xff8 [mtk_mdp3]
[    7.605519]  do_one_initcall+0x64/0x378
[    7.605526]  do_init_module+0xa4/0x2e0
[    7.605534]  load_module+0x24a8/0x2618
[    7.605541]  init_module_from_file+0x98/0x118
[    7.605547]  __arm64_sys_finit_module+0x284/0x380
[    7.605554]  invoke_syscall+0x74/0x128
[    7.605560]  el0_svc_common.constprop.0+0x114/0x140
[    7.605565]  do_el0_svc+0x28/0x58
[    7.605570]  el0_svc+0x44/0x1f0
[    7.605579]  el0t_64_sync_handler+0xc0/0x108
[    7.605586]  el0t_64_sync+0x1b8/0x1c0
[    7.605594] Code: d2800011 d65f03c0 b9808b22 910123e1 (b9400003)
[    7.605599] ---[ end trace 0000000000000000 ]---

Fixes: e6dec92308628 ("iommu/mediatek: Add mt2712 IOMMU support")
Signed-off-by: Jian Hui Lee <jianhui.lee@canonical.com>
---
 drivers/memory/mtk-smi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
index 733e22f695ab..8eb043ff8280 100644
--- a/drivers/memory/mtk-smi.c
+++ b/drivers/memory/mtk-smi.c
@@ -244,6 +244,10 @@ static int mtk_smi_larb_config_port_gen2_general(struct device *dev)
 	struct arm_smccc_res res;
 	int i;
 
+	/* larb->mmu and larb->bank are set in bind(), may not be ready yet */
+	if (!larb->mmu || !larb->bank)
+		return 0;
+
 	if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
 		return 0;
 
-- 
2.48.1



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

* Re: [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port
  2025-11-11 12:48 [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port Jian Hui Lee
@ 2025-11-12 15:51 ` Krzysztof Kozlowski
  2025-11-14  3:03   ` Jian Hui Lee
  0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-12 15:51 UTC (permalink / raw)
  To: Jian Hui Lee, Yong Wu, Matthias Brugger,
	AngeloGioacchino Del Regno, Joerg Roedel, linux-mediatek,
	linux-kernel, linux-arm-kernel

On 11/11/2025 13:48, Jian Hui Lee wrote:
> Fix a race condition where mtk_smi_larb_resume() can be called before
> mtk_smi_larb_bind() completes, leading to NULL pointer dereference when
> accessing larb->mmu and larb->bank.
> 
> Add NULL checks to skip port configuration gracefully when the device
> hasn't been bound yet.
> 
> Crash trace:
> [    7.605014] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
> [    7.605029] Mem abort info:
> [    7.605031]   ESR = 0x0000000096000004
> [    7.605034]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    7.605037]   SET = 0, FnV = 0
> [    7.605040]   EA = 0, S1PTW = 0
> [    7.605042]   FSC = 0x04: level 0 translation fault
> [    7.605045] Data abort info:
> [    7.605046]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
> [    7.605049]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [    7.605052]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [    7.605055] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000106755000
> [    7.605059] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000
> [    7.605069] Internal error: Oops: 0000000096000004 [#1]  SMP
> [    7.605075] Modules linked in: mtk_mdp3(+) snd_compress(+) ac97_bus snd_pcm_dmaengine snd_pcm v4l2_mem2mem videobufo
> [    7.605223]  pwm_mtk_disp spmi_mtk_pmif spmi_devres clk_mt8188_imp_iic_wrap spi_mt65xx mmc_hsq stmmac clk_mt8188_ado
> [    7.605267] CPU: 5 UID: 0 PID: 417 Comm: (udev-worker) Not tainted 6.17.2-custom #29 PREEMPT(voluntary)
> [    7.605273] Hardware name: mediatek Genio 700 EVK P1V4 (eMMC Boot)/Genio 700 EVK P1V4, BIOS 2022.10-gb56d2a20f5 10/2
> [    7.605278] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [    7.605283] pc : mtk_smi_larb_config_port_gen2_general+0x2a8/0x458 [mtk_smi]
> [    7.605301] lr : mtk_smi_larb_config_port_gen2_general+0x110/0x458 [mtk_smi]
> [    7.605307] sp : ffff8000867f3370
> [    7.605309] x29: ffff8000867f33e0 x28: 0000000000000000 x27: ffff80007c403fcc
> [    7.605317] x26: 000000000000000a x25: ffff0000d5107c80 x24: ffff80007c4040f8
> [    7.605325] x23: ffff80007c4040f8 x22: ffff80007c403fcc x21: ffff80007c43c918
> [    7.605332] x20: 0000000000000007 x19: ffff80008685d218 x18: ffff8000863b90e8
> [    7.605340] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [    7.605346] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [    7.605353] x11: 0000000000000000 x10: 0000000000000000 x9 : ffff80007c403fcc
> [    7.605360] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> [    7.605367] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000
> [    7.605373] x2 : 0000000000000000 x1 : ffff8000867f33b8 x0 : 0000000000000000
> [    7.605381] Call trace:
> [    7.605384]  mtk_smi_larb_config_port_gen2_general+0x2a8/0x458 [mtk_smi] (P)
> [    7.605393]  mtk_smi_larb_resume+0xbc/0x130 [mtk_smi]
> [    7.605399]  pm_generic_runtime_resume+0x38/0x80
> [    7.605410]  __genpd_runtime_resume+0x3c/0xb8
> [    7.605419]  genpd_runtime_resume+0xec/0x338
> [    7.605425]  __rpm_callback+0x54/0x210
> [    7.605431]  rpm_callback+0x94/0xa8
> [    7.605436]  rpm_resume+0x484/0x668
> [    7.605442]  __pm_runtime_resume+0x68/0xd8
> [    7.605447]  pm_runtime_get_suppliers+0x6c/0xb8
> [    7.605453]  __driver_probe_device+0x5c/0x1c8
> [    7.605461]  driver_probe_device+0x48/0x188
> [    7.605466]  __driver_attach+0x14c/0x2c8
> [    7.605471]  bus_for_each_dev+0x88/0x110
> [    7.605476]  driver_attach+0x30/0x60
> [    7.605481]  bus_add_driver+0x17c/0x2e8
> [    7.605486]  driver_register+0x68/0x178
> [    7.605492]  __platform_driver_register+0x30/0x60
> [    7.605498]  mdp_driver_init+0x2c/0xff8 [mtk_mdp3]
> [    7.605519]  do_one_initcall+0x64/0x378
> [    7.605526]  do_init_module+0xa4/0x2e0
> [    7.605534]  load_module+0x24a8/0x2618
> [    7.605541]  init_module_from_file+0x98/0x118
> [    7.605547]  __arm64_sys_finit_module+0x284/0x380
> [    7.605554]  invoke_syscall+0x74/0x128
> [    7.605560]  el0_svc_common.constprop.0+0x114/0x140
> [    7.605565]  do_el0_svc+0x28/0x58
> [    7.605570]  el0_svc+0x44/0x1f0
> [    7.605579]  el0t_64_sync_handler+0xc0/0x108
> [    7.605586]  el0t_64_sync+0x1b8/0x1c0
> [    7.605594] Code: d2800011 d65f03c0 b9808b22 910123e1 (b9400003)
> [    7.605599] ---[ end trace 0000000000000000 ]---

Please trim the traces from all irrelevant information.

> 
> Fixes: e6dec92308628 ("iommu/mediatek: Add mt2712 IOMMU support")

Add Cc stable.

> Signed-off-by: Jian Hui Lee <jianhui.lee@canonical.com>

> ---
>  drivers/memory/mtk-smi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> index 733e22f695ab..8eb043ff8280 100644
> --- a/drivers/memory/mtk-smi.c
> +++ b/drivers/memory/mtk-smi.c
> @@ -244,6 +244,10 @@ static int mtk_smi_larb_config_port_gen2_general(struct device *dev)
>  	struct arm_smccc_res res;
>  	int i;
>  
> +	/* larb->mmu and larb->bank are set in bind(), may not be ready yet */

And how do you synchronize this between CPUs? IOW, what certainty you
have that this CPU sees correct data and the checks below have any sense?

> +	if (!larb->mmu || !larb->bank)
> +		return 0;
> +
>  	if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
>  		return 0;
>  

Best regards,
Krzysztof


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

* Re: [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port
  2025-11-12 15:51 ` Krzysztof Kozlowski
@ 2025-11-14  3:03   ` Jian Hui Lee
  2025-11-14  7:21     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 5+ messages in thread
From: Jian Hui Lee @ 2025-11-14  3:03 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yong Wu, Matthias Brugger, AngeloGioacchino Del Regno,
	Joerg Roedel, linux-mediatek, linux-kernel, linux-arm-kernel

On Wed, Nov 12, 2025 at 11:51 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> Please trim the traces from all irrelevant information.
>
> >
> > Fixes: e6dec92308628 ("iommu/mediatek: Add mt2712 IOMMU support")
>
> Add Cc stable.
>
> > Signed-off-by: Jian Hui Lee <jianhui.lee@canonical.com>
>
> > ---
> >  drivers/memory/mtk-smi.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> > index 733e22f695ab..8eb043ff8280 100644
> > --- a/drivers/memory/mtk-smi.c
> > +++ b/drivers/memory/mtk-smi.c
> > @@ -244,6 +244,10 @@ static int mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >       struct arm_smccc_res res;
> >       int i;
> >
> > +     /* larb->mmu and larb->bank are set in bind(), may not be ready yet */
>
> And how do you synchronize this between CPUs? IOW, what certainty you
> have that this CPU sees correct data and the checks below have any sense?

Thank you for the review and for pointing this out. Moving
pm_runtime_enable/disable to component bind/unbind should ensure that
the synchronization happens correctly. I will send a v2 to address
those you mentioned.

> > +     if (!larb->mmu || !larb->bank)
> > +             return 0;
> > +
> >       if (BIT(larb->larbid) & larb->larb_gen->larb_direct_to_common_mask)
> >               return 0;
> >
>
> Best regards,
> Krzysztof


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

* Re: [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port
  2025-11-14  3:03   ` Jian Hui Lee
@ 2025-11-14  7:21     ` Krzysztof Kozlowski
  2025-11-18  6:15       ` Jian Hui Lee
  0 siblings, 1 reply; 5+ messages in thread
From: Krzysztof Kozlowski @ 2025-11-14  7:21 UTC (permalink / raw)
  To: Jian Hui Lee
  Cc: Yong Wu, Matthias Brugger, AngeloGioacchino Del Regno,
	Joerg Roedel, linux-mediatek, linux-kernel, linux-arm-kernel

On 14/11/2025 04:03, Jian Hui Lee wrote:
> On Wed, Nov 12, 2025 at 11:51 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>
>> Please trim the traces from all irrelevant information.
>>
>>>
>>> Fixes: e6dec92308628 ("iommu/mediatek: Add mt2712 IOMMU support")
>>
>> Add Cc stable.
>>
>>> Signed-off-by: Jian Hui Lee <jianhui.lee@canonical.com>
>>
>>> ---
>>>  drivers/memory/mtk-smi.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
>>> index 733e22f695ab..8eb043ff8280 100644
>>> --- a/drivers/memory/mtk-smi.c
>>> +++ b/drivers/memory/mtk-smi.c
>>> @@ -244,6 +244,10 @@ static int mtk_smi_larb_config_port_gen2_general(struct device *dev)
>>>       struct arm_smccc_res res;
>>>       int i;
>>>
>>> +     /* larb->mmu and larb->bank are set in bind(), may not be ready yet */
>>
>> And how do you synchronize this between CPUs? IOW, what certainty you
>> have that this CPU sees correct data and the checks below have any sense?
> 
> Thank you for the review and for pointing this out. Moving
> pm_runtime_enable/disable to component bind/unbind should ensure that
> the synchronization happens correctly. I will send a v2 to address
> those you mentioned.

Runtime PM synchronizes nothing in this matter, so I don't understand
what you want to achieve with it.

Best regards,
Krzysztof


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

* Re: [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port
  2025-11-14  7:21     ` Krzysztof Kozlowski
@ 2025-11-18  6:15       ` Jian Hui Lee
  0 siblings, 0 replies; 5+ messages in thread
From: Jian Hui Lee @ 2025-11-18  6:15 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Yong Wu, Matthias Brugger, AngeloGioacchino Del Regno,
	Joerg Roedel, linux-mediatek, linux-kernel, linux-arm-kernel

On Fri, Nov 14, 2025 at 3:21 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On 14/11/2025 04:03, Jian Hui Lee wrote:
> > On Wed, Nov 12, 2025 at 11:51 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >>
> >> Please trim the traces from all irrelevant information.
> >>
> >>>
> >>> Fixes: e6dec92308628 ("iommu/mediatek: Add mt2712 IOMMU support")
> >>
> >> Add Cc stable.
> >>
> >>> Signed-off-by: Jian Hui Lee <jianhui.lee@canonical.com>
> >>
> >>> ---
> >>>  drivers/memory/mtk-smi.c | 4 ++++
> >>>  1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/drivers/memory/mtk-smi.c b/drivers/memory/mtk-smi.c
> >>> index 733e22f695ab..8eb043ff8280 100644
> >>> --- a/drivers/memory/mtk-smi.c
> >>> +++ b/drivers/memory/mtk-smi.c
> >>> @@ -244,6 +244,10 @@ static int mtk_smi_larb_config_port_gen2_general(struct device *dev)
> >>>       struct arm_smccc_res res;
> >>>       int i;
> >>>
> >>> +     /* larb->mmu and larb->bank are set in bind(), may not be ready yet */
> >>
> >> And how do you synchronize this between CPUs? IOW, what certainty you
> >> have that this CPU sees correct data and the checks below have any sense?
> >
> > Thank you for the review and for pointing this out. Moving
> > pm_runtime_enable/disable to component bind/unbind should ensure that
> > the synchronization happens correctly. I will send a v2 to address
> > those you mentioned.
>
> Runtime PM synchronizes nothing in this matter, so I don't understand
> what you want to achieve with it.

The original issue was a race condition where PM runtime resume could
be triggered before the device was fully initialized, causing NULL
pointer dereferences. So moving pm_runtime_enable from
mtk_smi_larb_probe to mtk_smi_larb_bind with the barrier presumably
fixes the issue:

@@ -171,6 +171,9 @@ mtk_smi_larb_bind(struct device *dev, struct
device *master, void *data)
                        larb->larbid = i;
                        larb->mmu = &larb_mmu[i].mmu;
                        larb->bank = larb_mmu[i].bank;
+                       smp_wmb();
+                       pm_runtime_enable(dev);
                        return 0;
                }
@@ -664,15 +668,13 @@ static int mtk_smi_larb_probe(struct
platform_device *pdev)
        if (ret < 0)
                return ret;

-       pm_runtime_enable(dev);
        platform_set_drvdata(pdev, larb);
        ret = component_add(dev, &mtk_smi_larb_component_ops);
        if (ret)

> Best regards,
> Krzysztof


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

end of thread, other threads:[~2025-11-18  6:15 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11 12:48 [PATCH] memory: mtk-smi: Fix NULL pointer dereference in config_port Jian Hui Lee
2025-11-12 15:51 ` Krzysztof Kozlowski
2025-11-14  3:03   ` Jian Hui Lee
2025-11-14  7:21     ` Krzysztof Kozlowski
2025-11-18  6:15       ` Jian Hui Lee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).