Re: [PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
Hi Yong,
>On Mon, 2022-04-25 at 11:03 +0100, Robin Murphy wrote:
>> On 2022-04-25 09:24, Miles Chen via iommu wrote:
>> > When larbdev is NULL (in the case I hit, the node is incorrectly
>> > set
>> > iommus = < NUM>), it will cause device_link_add() fail and
>>
>> Until the MT8195 infra MMU support lands, is there ever a case where
>> it's actually valid for larbdev to be NULL? If not, I think it would
>> be
>> a lot clearer to explicitly fail the probe here, rather than
>> silently
>> continue and risk fatal errors, hangs, or other weird behaviour if
>> there's no guarantee that the correct LARB is powered up (plus then
>> the
>> release callbacks wouldn't need to worry about it either).
>
>Yes. It should return fail for this case. This issue only happens when
>the dts parameters doesn't respect the definition from the binding[1].
>
>Locally Miles tested with a internal definition that have not send
>upstream to get this KE. In this case, I'm not sure if we should
>request the user use the right ID in dts. Anyway I have no objection to
>modifying this, then something like this: (Avoid invalid input from
>dtb)
>
>@@ -790,6 +790,8 @@ static struct iommu_device
>*mtk_iommu_probe_device(struct device *dev)
>* All the ports in each a device should be in the same larbs.
>*/
> larbid = MTK_M4U_TO_LARB(fwspec->ids[0]);
>+ if (larbid >= MTK_LARB_NR_MAX)
>+ return ERR_PTR(-EINVAL);
> for (i = 1; i < fwspec->num_ids; i++) {
> larbidx = MTK_M4U_TO_LARB(fwspec->ids[i]);
> if (larbid != larbidx) {
>@@ -799,6 +801,8 @@ static struct iommu_device
>*mtk_iommu_probe_device(struct device *dev)
> }
> }
> larbdev = data->larb_imu[larbid].dev;
>+ if (!larbdev)
>+ return ERR_PTR(-EINVAL);
> link = device_link_add(dev, larbdev,
> DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
> if (!link)
Thanks for guilding me, I will put this in patch v2.
Thanks,
Miles
>
>
>[1]
>https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/iommu/mediatek,iommu.yaml#L116
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
hi Robin,
>> -link = device_link_add(dev, larbdev,
>> - DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
>> -if (!link)
>> -dev_err(dev, "Unable to link %s\n", dev_name(larbdev));
>> +if (larbdev) {
>
>Until the MT8195 infra MMU support lands, is there ever a case where
>it's actually valid for larbdev to be NULL? If not, I think it would be
>a lot clearer to explicitly fail the probe here, rather than silently
>continue and risk fatal errors, hangs, or other weird behaviour if
>there's no guarantee that the correct LARB is powered up (plus then the
>release callbacks wouldn't need to worry about it either).
Thanks, I will do probe fail in patch v3 and remove the release modification.
thanks,
Miles
>
>Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
On Mon, 2022-04-25 at 11:03 +0100, Robin Murphy wrote:
> On 2022-04-25 09:24, Miles Chen via iommu wrote:
> > When larbdev is NULL (in the case I hit, the node is incorrectly
> > set
> > iommus = < NUM>), it will cause device_link_add() fail and
> > kernel crashes when we try to print dev_name(larbdev).
> >
> > Fix it by adding a NULL pointer check before
> > device_link_add/device_link_remove.
> >
> > It should work for normal correct setting and avoid the crash
> > caused
> > by my incorrect setting.
> >
> > Error log:
> > [ 18.189042][ T301] Unable to handle kernel NULL pointer
> > dereference at virtual address 0050
> > [ 18.190247][ T301] Mem abort info:
> > [ 18.190255][ T301] ESR = 0x9605
> > [ 18.190263][ T301] EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 18.192142][ T301] SET = 0, FnV = 0
> > [ 18.192151][ T301] EA = 0, S1PTW = 0
> > [ 18.194710][ T301] FSC = 0x05: level 1 translation fault
> > [ 18.195424][ T301] Data abort info:
> > [ 18.195888][ T301] ISV = 0, ISS = 0x0005
> > [ 18.196500][ T301] CM = 0, WnR = 0
> > [ 18.196977][ T301] user pgtable: 4k pages, 39-bit VAs,
> > pgdp=000104f9e000
> > [ 18.197889][ T301] [0050] pgd=,
> > p4d=, pud=
> > [ 18.199220][ T301] Internal error: Oops: 9605 [#1] PREEMPT
> > SMP
> > [ 18.343152][ T301] Kernel Offset: 0x144408 from
> > 0xffc00800
> > [ 18.343988][ T301] PHYS_OFFSET: 0x4000
> > [ 18.344519][ T301] pstate: a045 (NzCv daif +PAN -UAO)
> > [ 18.345213][ T301] pc : mtk_iommu_probe_device+0xf8/0x118
> > [mtk_iommu]
> > [ 18.346050][ T301] lr : mtk_iommu_probe_device+0xd0/0x118
> > [mtk_iommu]
> > [ 18.346884][ T301] sp : ffc00a5635e0
> > [ 18.347392][ T301] x29: ffc00a5635e0 x28: ffd44a46c1d8
> > [ 18.348156][ T301] x27: ff80c39a8000 x26: ffd44a80cc38
> > [ 18.348917][ T301] x25: x24: ffd44a80cc38
> > [ 18.349677][ T301] x23: ffd44e4da4c6 x22: ffd44a80cc38
> > [ 18.350438][ T301] x21: ff80cecd1880 x20:
> > [ 18.351198][ T301] x19: ff80c439f010 x18: ffc00a50d0c0
> > [ 18.351959][ T301] x17: x16: 0004
> > [ 18.352719][ T301] x15: 0004 x14: ffd44eb5d420
> > [ 18.353480][ T301] x13: 0ad2 x12: 0003
> > [ 18.354241][ T301] x11: fad2 x10: c000fad2
> > [ 18.355003][ T301] x9 : a0d288d8d7142d00 x8 : a0d288d8d7142d00
> > [ 18.355763][ T301] x7 : ffd44c2bc640 x6 :
> > [ 18.356524][ T301] x5 : 0080 x4 : 0001
> > [ 18.357284][ T301] x3 : x2 : 0005
> > [ 18.358045][ T301] x1 : x0 :
> > [ 18.360208][ T301] Hardware name: MT6873 (DT)
> > [ 18.360771][ T301] Call trace:
> > [ 18.361168][ T301] dump_backtrace+0xf8/0x1f0
> > [ 18.361737][ T301] dump_stack_lvl+0xa8/0x11c
> > [ 18.362305][ T301] dump_stack+0x1c/0x2c
> > [ 18.362816][ T301] mrdump_common_die+0x184/0x40c [mrdump]
> > [ 18.363575][ T301] ipanic_die+0x24/0x38 [mrdump]
> > [ 18.364230][ T301] atomic_notifier_call_chain+0x128/0x2b8
> > [ 18.364937][ T301] die+0x16c/0x568
> > [ 18.365394][ T301] __do_kernel_fault+0x1e8/0x214
> > [ 18.365402][ T301] do_page_fault+0xb8/0x678
> > [ 18.366934][ T301] do_translation_fault+0x48/0x64
> > [ 18.368645][ T301] do_mem_abort+0x68/0x148
> > [ 18.368652][ T301] el1_abort+0x40/0x64
> > [ 18.368660][ T301] el1h_64_sync_handler+0x54/0x88
> > [ 18.368668][ T301] el1h_64_sync+0x68/0x6c
> > [ 18.368673][ T301] mtk_iommu_probe_device+0xf8/0x118
> > [mtk_iommu]
> > [ 18.369840][ T301] __iommu_probe_device+0x12c/0x358
> > [ 18.370880][ T301] iommu_probe_device+0x3c/0x31c
> > [ 18.372026][ T301] of_iommu_configure+0x200/0x274
> > [ 18.373587][ T301] of_dma_configure_id+0x1b8/0x230
> > [ 18.375200][ T301] platform_dma_configure+0x24/0x3c
> > [ 18.376456][ T301] really_probe+0x110/0x504
> > [ 18.376464][ T301] __driver_probe_device+0xb4/0x188
> > [ 18.376472][ T301] driver_probe_device+0x5c/0x2b8
> > [ 18.376481][ T301] __driver_attach+0x338/0x42c
> > [ 18.377992][ T301] bus_add_driver+0x218/0x4c8
> > [ 18.379389][ T301] driver_register+0x84/0x17c
> > [ 18.380580][ T301] __platform_driver_register+0x28/0x38
> > ...
> >
> > Reported-by: kernel test robot
> > Fixes: 635319a4a744 ("media: iommu/mediatek: Add device_link
> > between the consumer and the larb devices")
> > Signed-off-by: Miles Chen
> >
> > ---
> >
> > Change since v1
> > fix a build warning reported by kernel test robot
> > https://lore.kernel.org/lkml/202204231446.iykdz674-...@intel.com/
> >
> > ---
> > drivers/iommu/mtk_iommu.c| 13 -
> > drivers/iommu/mtk_iommu_v1.c | 13 -
> >
Re: [PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
On 2022-04-25 09:24, Miles Chen via iommu wrote:
When larbdev is NULL (in the case I hit, the node is incorrectly set
iommus = < NUM>), it will cause device_link_add() fail and
kernel crashes when we try to print dev_name(larbdev).
Fix it by adding a NULL pointer check before
device_link_add/device_link_remove.
It should work for normal correct setting and avoid the crash caused
by my incorrect setting.
Error log:
[ 18.189042][ T301] Unable to handle kernel NULL pointer dereference at
virtual address 0050
[ 18.190247][ T301] Mem abort info:
[ 18.190255][ T301] ESR = 0x9605
[ 18.190263][ T301] EC = 0x25: DABT (current EL), IL = 32 bits
[ 18.192142][ T301] SET = 0, FnV = 0
[ 18.192151][ T301] EA = 0, S1PTW = 0
[ 18.194710][ T301] FSC = 0x05: level 1 translation fault
[ 18.195424][ T301] Data abort info:
[ 18.195888][ T301] ISV = 0, ISS = 0x0005
[ 18.196500][ T301] CM = 0, WnR = 0
[ 18.196977][ T301] user pgtable: 4k pages, 39-bit VAs, pgdp=000104f9e000
[ 18.197889][ T301] [0050] pgd=,
p4d=, pud=
[ 18.199220][ T301] Internal error: Oops: 9605 [#1] PREEMPT SMP
[ 18.343152][ T301] Kernel Offset: 0x144408 from 0xffc00800
[ 18.343988][ T301] PHYS_OFFSET: 0x4000
[ 18.344519][ T301] pstate: a045 (NzCv daif +PAN -UAO)
[ 18.345213][ T301] pc : mtk_iommu_probe_device+0xf8/0x118 [mtk_iommu]
[ 18.346050][ T301] lr : mtk_iommu_probe_device+0xd0/0x118 [mtk_iommu]
[ 18.346884][ T301] sp : ffc00a5635e0
[ 18.347392][ T301] x29: ffc00a5635e0 x28: ffd44a46c1d8
[ 18.348156][ T301] x27: ff80c39a8000 x26: ffd44a80cc38
[ 18.348917][ T301] x25: x24: ffd44a80cc38
[ 18.349677][ T301] x23: ffd44e4da4c6 x22: ffd44a80cc38
[ 18.350438][ T301] x21: ff80cecd1880 x20:
[ 18.351198][ T301] x19: ff80c439f010 x18: ffc00a50d0c0
[ 18.351959][ T301] x17: x16: 0004
[ 18.352719][ T301] x15: 0004 x14: ffd44eb5d420
[ 18.353480][ T301] x13: 0ad2 x12: 0003
[ 18.354241][ T301] x11: fad2 x10: c000fad2
[ 18.355003][ T301] x9 : a0d288d8d7142d00 x8 : a0d288d8d7142d00
[ 18.355763][ T301] x7 : ffd44c2bc640 x6 :
[ 18.356524][ T301] x5 : 0080 x4 : 0001
[ 18.357284][ T301] x3 : x2 : 0005
[ 18.358045][ T301] x1 : x0 :
[ 18.360208][ T301] Hardware name: MT6873 (DT)
[ 18.360771][ T301] Call trace:
[ 18.361168][ T301] dump_backtrace+0xf8/0x1f0
[ 18.361737][ T301] dump_stack_lvl+0xa8/0x11c
[ 18.362305][ T301] dump_stack+0x1c/0x2c
[ 18.362816][ T301] mrdump_common_die+0x184/0x40c [mrdump]
[ 18.363575][ T301] ipanic_die+0x24/0x38 [mrdump]
[ 18.364230][ T301] atomic_notifier_call_chain+0x128/0x2b8
[ 18.364937][ T301] die+0x16c/0x568
[ 18.365394][ T301] __do_kernel_fault+0x1e8/0x214
[ 18.365402][ T301] do_page_fault+0xb8/0x678
[ 18.366934][ T301] do_translation_fault+0x48/0x64
[ 18.368645][ T301] do_mem_abort+0x68/0x148
[ 18.368652][ T301] el1_abort+0x40/0x64
[ 18.368660][ T301] el1h_64_sync_handler+0x54/0x88
[ 18.368668][ T301] el1h_64_sync+0x68/0x6c
[ 18.368673][ T301] mtk_iommu_probe_device+0xf8/0x118 [mtk_iommu]
[ 18.369840][ T301] __iommu_probe_device+0x12c/0x358
[ 18.370880][ T301] iommu_probe_device+0x3c/0x31c
[ 18.372026][ T301] of_iommu_configure+0x200/0x274
[ 18.373587][ T301] of_dma_configure_id+0x1b8/0x230
[ 18.375200][ T301] platform_dma_configure+0x24/0x3c
[ 18.376456][ T301] really_probe+0x110/0x504
[ 18.376464][ T301] __driver_probe_device+0xb4/0x188
[ 18.376472][ T301] driver_probe_device+0x5c/0x2b8
[ 18.376481][ T301] __driver_attach+0x338/0x42c
[ 18.377992][ T301] bus_add_driver+0x218/0x4c8
[ 18.379389][ T301] driver_register+0x84/0x17c
[ 18.380580][ T301] __platform_driver_register+0x28/0x38
...
Reported-by: kernel test robot
Fixes: 635319a4a744 ("media: iommu/mediatek: Add device_link between the consumer
and the larb devices")
Signed-off-by: Miles Chen
---
Change since v1
fix a build warning reported by kernel test robot
https://lore.kernel.org/lkml/202204231446.iykdz674-...@intel.com/
---
drivers/iommu/mtk_iommu.c| 13 -
drivers/iommu/mtk_iommu_v1.c | 13 -
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6fd75a60abd6..03e0133f346a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -581,10 +581,12 @@ static struct iommu_device *mtk_iommu_probe_device(struct
device *dev)
}
}
larbdev = data->larb_imu[larbid].dev;
- link = device_link_add(dev, larbdev,
-
[PATCH v2] iommu/mediatek: fix NULL pointer dereference when printing dev_name
When larbdev is NULL (in the case I hit, the node is incorrectly set
iommus = < NUM>), it will cause device_link_add() fail and
kernel crashes when we try to print dev_name(larbdev).
Fix it by adding a NULL pointer check before
device_link_add/device_link_remove.
It should work for normal correct setting and avoid the crash caused
by my incorrect setting.
Error log:
[ 18.189042][ T301] Unable to handle kernel NULL pointer dereference at
virtual address 0050
[ 18.190247][ T301] Mem abort info:
[ 18.190255][ T301] ESR = 0x9605
[ 18.190263][ T301] EC = 0x25: DABT (current EL), IL = 32 bits
[ 18.192142][ T301] SET = 0, FnV = 0
[ 18.192151][ T301] EA = 0, S1PTW = 0
[ 18.194710][ T301] FSC = 0x05: level 1 translation fault
[ 18.195424][ T301] Data abort info:
[ 18.195888][ T301] ISV = 0, ISS = 0x0005
[ 18.196500][ T301] CM = 0, WnR = 0
[ 18.196977][ T301] user pgtable: 4k pages, 39-bit VAs, pgdp=000104f9e000
[ 18.197889][ T301] [0050] pgd=,
p4d=, pud=
[ 18.199220][ T301] Internal error: Oops: 9605 [#1] PREEMPT SMP
[ 18.343152][ T301] Kernel Offset: 0x144408 from 0xffc00800
[ 18.343988][ T301] PHYS_OFFSET: 0x4000
[ 18.344519][ T301] pstate: a045 (NzCv daif +PAN -UAO)
[ 18.345213][ T301] pc : mtk_iommu_probe_device+0xf8/0x118 [mtk_iommu]
[ 18.346050][ T301] lr : mtk_iommu_probe_device+0xd0/0x118 [mtk_iommu]
[ 18.346884][ T301] sp : ffc00a5635e0
[ 18.347392][ T301] x29: ffc00a5635e0 x28: ffd44a46c1d8
[ 18.348156][ T301] x27: ff80c39a8000 x26: ffd44a80cc38
[ 18.348917][ T301] x25: x24: ffd44a80cc38
[ 18.349677][ T301] x23: ffd44e4da4c6 x22: ffd44a80cc38
[ 18.350438][ T301] x21: ff80cecd1880 x20:
[ 18.351198][ T301] x19: ff80c439f010 x18: ffc00a50d0c0
[ 18.351959][ T301] x17: x16: 0004
[ 18.352719][ T301] x15: 0004 x14: ffd44eb5d420
[ 18.353480][ T301] x13: 0ad2 x12: 0003
[ 18.354241][ T301] x11: fad2 x10: c000fad2
[ 18.355003][ T301] x9 : a0d288d8d7142d00 x8 : a0d288d8d7142d00
[ 18.355763][ T301] x7 : ffd44c2bc640 x6 :
[ 18.356524][ T301] x5 : 0080 x4 : 0001
[ 18.357284][ T301] x3 : x2 : 0005
[ 18.358045][ T301] x1 : x0 :
[ 18.360208][ T301] Hardware name: MT6873 (DT)
[ 18.360771][ T301] Call trace:
[ 18.361168][ T301] dump_backtrace+0xf8/0x1f0
[ 18.361737][ T301] dump_stack_lvl+0xa8/0x11c
[ 18.362305][ T301] dump_stack+0x1c/0x2c
[ 18.362816][ T301] mrdump_common_die+0x184/0x40c [mrdump]
[ 18.363575][ T301] ipanic_die+0x24/0x38 [mrdump]
[ 18.364230][ T301] atomic_notifier_call_chain+0x128/0x2b8
[ 18.364937][ T301] die+0x16c/0x568
[ 18.365394][ T301] __do_kernel_fault+0x1e8/0x214
[ 18.365402][ T301] do_page_fault+0xb8/0x678
[ 18.366934][ T301] do_translation_fault+0x48/0x64
[ 18.368645][ T301] do_mem_abort+0x68/0x148
[ 18.368652][ T301] el1_abort+0x40/0x64
[ 18.368660][ T301] el1h_64_sync_handler+0x54/0x88
[ 18.368668][ T301] el1h_64_sync+0x68/0x6c
[ 18.368673][ T301] mtk_iommu_probe_device+0xf8/0x118 [mtk_iommu]
[ 18.369840][ T301] __iommu_probe_device+0x12c/0x358
[ 18.370880][ T301] iommu_probe_device+0x3c/0x31c
[ 18.372026][ T301] of_iommu_configure+0x200/0x274
[ 18.373587][ T301] of_dma_configure_id+0x1b8/0x230
[ 18.375200][ T301] platform_dma_configure+0x24/0x3c
[ 18.376456][ T301] really_probe+0x110/0x504
[ 18.376464][ T301] __driver_probe_device+0xb4/0x188
[ 18.376472][ T301] driver_probe_device+0x5c/0x2b8
[ 18.376481][ T301] __driver_attach+0x338/0x42c
[ 18.377992][ T301] bus_add_driver+0x218/0x4c8
[ 18.379389][ T301] driver_register+0x84/0x17c
[ 18.380580][ T301] __platform_driver_register+0x28/0x38
...
Reported-by: kernel test robot
Fixes: 635319a4a744 ("media: iommu/mediatek: Add device_link between the
consumer and the larb devices")
Signed-off-by: Miles Chen
---
Change since v1
fix a build warning reported by kernel test robot
https://lore.kernel.org/lkml/202204231446.iykdz674-...@intel.com/
---
drivers/iommu/mtk_iommu.c| 13 -
drivers/iommu/mtk_iommu_v1.c | 13 -
2 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6fd75a60abd6..03e0133f346a 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -581,10 +581,12 @@ static struct iommu_device *mtk_iommu_probe_device(struct
device *dev)
}
}
larbdev = data->larb_imu[larbid].dev;
- link = device_link_add(dev, larbdev,
- DL_FLAG_PM_RUNTIME | DL_FLAG_STATELESS);
