diff mbox series

[v3,06/15] arm64: dts: qcom: sc8280xp: Fix the base addresses of LLCC banks

Message ID 20221219182958.476231-7-manivannan.sadhasivam@linaro.org
State Superseded
Headers show
Series Qcom: LLCC/EDAC: Fix base address used for LLCC banks | expand

Commit Message

Manivannan Sadhasivam Dec. 19, 2022, 6:29 p.m. UTC
The LLCC block has several banks each with a different base address
and holes in between. So it is not a correct approach to cover these
banks with a single offset/size. Instead, the individual bank's base
address needs to be specified in devicetree with the exact size.

Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Steev Klimaszewski Dec. 20, 2022, 4:56 a.m. UTC | #1
On Mon, Dec 19, 2022 at 12:31 PM Manivannan Sadhasivam
<manivannan.sadhasivam@linaro.org> wrote:
>
> The LLCC block has several banks each with a different base address
> and holes in between. So it is not a correct approach to cover these
> banks with a single offset/size. Instead, the individual bank's base
> address needs to be specified in devicetree with the exact size.
>
> Reported-by: Parikshit Pareek <quic_ppareek@quicinc.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp.dtsi | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> index 109c9d2b684d..0510a5d510e7 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
> @@ -1856,8 +1856,14 @@ opp-6 {
>
>                 system-cache-controller@9200000 {
>                         compatible = "qcom,sc8280xp-llcc";
> -                       reg = <0 0x09200000 0 0x58000>, <0 0x09600000 0 0x58000>;
> -                       reg-names = "llcc_base", "llcc_broadcast_base";
> +                       reg = <0 0x09200000 0 0x58000>, <0 0x09280000 0 0x58000>,
> +                             <0 0x09300000 0 0x58000>, <0 0x09380000 0 0x58000>,
> +                             <0 0x09400000 0 0x58000>, <0 0x09480000 0 0x58000>,
> +                             <0 0x09500000 0 0x58000>, <0 0x09580000 0 0x58000>,
> +                             <0 0x09600000 0 0x58000>;
> +                       reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
> +                                   "llcc3_base", "llcc4_base", "llcc5_base",
> +                                   "llcc6_base", "llcc7_base",  "llcc_broadcast_base";
>                         interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
>                 };
>
> --
> 2.25.1
>
Hi Mani,

Replying on this one, because I tested v3 on the Thinkpad X13s here...
The module probes fine, and like Andrew I see it loading...

But there still seem to be some issues...

I installed edac-utils, and I had to edit /etc/default/edac to
actually tell it to use the qcom_edac module, since it could not
figure this out on its own.
Restarting the edac service, seemed to go okay, but *stopping* it,
which unloads the module (though restarting did the same thing, so I'm
not sure why it succeeded the first time and not the second) and we
get:

[ 8470.972150] EDAC MC: Removed device 0 for qcom_llcc_edac llcc: DEV
qcom_llcc_edac
[ 8471.047124] EDAC DEVICE1: Giving out device to module
qcom_llcc_edac controller llcc: DEV qcom_llcc_edac (INTERRUPT)
[ 8477.005625] EDAC MC: Removed device 1 for qcom_llcc_edac llcc: DEV
qcom_llcc_edac
[ 8477.005659] ------------[ cut here ]------------
[ 8477.005661] kernel BUG at mm/slub.c:435!
[ 8477.005668] Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
[ 8477.005674] Modules linked in: qcom_edac(-) aes_ce_ccm michael_mic
snd_soc_wsa883x q6prm_clocks q6apm_lpass_dais snd_q6dsp_common
q6apm_dai q6prm qrtr_mhi snd_seq_dummy snd_hrtimer snd_seq
snd_seq_device nls_ascii nls_cp437 vfat fat sg snd_q6apm qrtr_smd
pm8941_pwrkey overlay aes_ce_blk aes_ce_cipher ghash_ce gf128mul
sha2_ce qcom_spmi_adc_tm5 qcom_spmi_adc5 sha256_arm64 qcom_vadc_common
qcom_pon sha1_ce snd_soc_wcd938x qcom_spmi_temp_alarm regmap_sdw
ath11k_pci snd_soc_sc8280xp snd_soc_wcd938x_sdw industrialio
snd_soc_hdmi_codec ath11k snd_soc_qcom_common snd_soc_wcd_mbhc
mac80211 snd_soc_lpass_rx_macro snd_soc_lpass_va_macro
snd_soc_lpass_tx_macro soundwire_qcom snd_soc_lpass_macro_common
snd_soc_lpass_wsa_macro libarc4 snd_soc_core snd_compress
soundwire_bus snd_pcm_dmaengine cfg80211 snd_pcm qcom_q6v5_pas mhi
qcom_pil_info qcom_q6v5 snd_timer qcom_sysmon snd qcom_common slimbus
soundcore qcom_rng qcom_battmgr joydev hid_multitouch evdev
binfmt_misc fuse configfs ip_tables x_tables
[ 8477.005791] autofs4 ext4 mbcache jbd2 uas msm mdt_loader ocmem
gpu_sched llcc_qcom gpio_keys qrtr [last unloaded: qcom_edac]
[ 8477.005814] CPU: 7 PID: 3215 Comm: modprobe Not tainted
6.1.0-next-20221219 #25
[ 8477.005818] Hardware name: LENOVO 21BX0015US/21BX0015US, BIOS
N3HET74W (1.46 ) 10/12/2022
[ 8477.005822] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 8477.005826] pc : __slab_free+0x118/0x550
[ 8477.005839] lr : __slab_free+0x48/0x550
[ 8477.005847] sp : ffff80000ca1ba70
[ 8477.005849] x29: ffff80000ca1ba70 x28: ffff1b0bc0002200 x27: 0000000000000000
[ 8477.005855] x26: 0000000000e46b07 x25: 0000000000000000 x24: ffff1b133bf3e800
[ 8477.005862] x23: 000000000020001f x22: ffff1b133bf3e800 x21: ffffaf4f127f0470
[ 8477.005867] x20: ffff1b133bf3e800 x19: fffffc6c4cefcf80 x18: ffffffffffffffff
[ 8477.005873] x17: 63636c6c5f6d6f63 x16: ffffaf4f127ee274 x15: 636c6c2063616465
[ 8477.005878] x14: 0000000000000004 x13: ffff1b0bc0271810 x12: 0000000000000000
[ 8477.005884] x11: ffff1b0bfe2d2720 x10: ffff1b0bfe2d26c8 x9 : ffffaf4f120cda00
[ 8477.005889] x8 : ffff80000ca1bb40 x7 : 0000000000000000 x6 : 0000000000000000
[ 8477.005894] x5 : ffffaf4f127f0470 x4 : 0000000000000000 x3 : ffff1b133bf3e840
[ 8477.005899] x2 : ffffffffffffffc0 x1 : 0000000000000000 x0 : 0000000000000040
[ 8477.005905] Call trace:
[ 8477.005907] __slab_free+0x118/0x550
[ 8477.005914] __kmem_cache_free+0x290/0x2b4
[ 8477.005918] kfree+0x8c/0x174
[ 8477.005925] edac_device_ctrl_master_release+0x30/0x60
[ 8477.005933] kobject_put+0xb0/0x220
[ 8477.005940] edac_device_unregister_sysfs_main_kobj+0x1c/0x30
[ 8477.005944] edac_device_free_ctl_info+0x18/0x2c
[ 8477.005950] qcom_llcc_edac_remove+0x2c/0x40 [qcom_edac]
[ 8477.005962] platform_remove+0x30/0x64
[ 8477.005969] device_remove+0x54/0x8c
[ 8477.005973] device_release_driver_internal+0x1ec/0x260
[ 8477.005978] driver_detach+0x58/0xa0
[ 8477.005983] bus_remove_driver+0x64/0x100
[ 8477.005988] driver_unregister+0x38/0x70
[ 8477.005993] platform_driver_unregister+0x1c/0x30
[ 8477.005998] qcom_llcc_edac_driver_exit+0x18/0x918 [qcom_edac]
[ 8477.006007] __arm64_sys_delete_module+0x1c0/0x340
[ 8477.006014] invoke_syscall+0x50/0x120
[ 8477.006021] el0_svc_common.constprop.0+0x4c/0xf4
[ 8477.006026] do_el0_svc+0x40/0xbc
[ 8477.006032] el0_svc+0x2c/0x84
[ 8477.006038] el0t_64_sync_handler+0xf4/0x120
[ 8477.006043] el0t_64_sync+0x190/0x194
[ 8477.006049] Code: b9402b80 8b000283 eb1402df 54fffb01 (d4210000)
[ 8477.006053] ---[ end trace 0000000000000000 ]---
[ 8477.006056] note: modprobe[3215] exited with preempt_count 1
[ 8477.006282] ------------[ cut here ]------------
[ 8477.006285] WARNING: CPU: 7 PID: 0 at kernel/context_tracking.c:128
ct_kernel_exit.constprop.0+0xe0/0xf0
[ 8477.006294] Modules linked in: qcom_edac(-) aes_ce_ccm michael_mic
snd_soc_wsa883x q6prm_clocks q6apm_lpass_dais snd_q6dsp_common
q6apm_dai q6prm qrtr_mhi snd_seq_dummy snd_hrtimer snd_seq
snd_seq_device nls_ascii nls_cp437 vfat fat sg snd_q6apm qrtr_smd
pm8941_pwrkey overlay aes_ce_blk aes_ce_cipher ghash_ce gf128mul
sha2_ce qcom_spmi_adc_tm5 qcom_spmi_adc5 sha256_arm64 qcom_vadc_common
qcom_pon sha1_ce snd_soc_wcd938x qcom_spmi_temp_alarm regmap_sdw
ath11k_pci snd_soc_sc8280xp snd_soc_wcd938x_sdw industrialio
snd_soc_hdmi_codec ath11k snd_soc_qcom_common snd_soc_wcd_mbhc
mac80211 snd_soc_lpass_rx_macro snd_soc_lpass_va_macro
snd_soc_lpass_tx_macro soundwire_qcom snd_soc_lpass_macro_common
snd_soc_lpass_wsa_macro libarc4 snd_soc_core snd_compress
soundwire_bus snd_pcm_dmaengine cfg80211 snd_pcm qcom_q6v5_pas mhi
qcom_pil_info qcom_q6v5 snd_timer qcom_sysmon snd qcom_common slimbus
soundcore qcom_rng qcom_battmgr joydev hid_multitouch evdev
binfmt_misc fuse configfs ip_tables x_tables
[ 8477.006378] autofs4 ext4 mbcache jbd2 uas msm mdt_loader ocmem
gpu_sched llcc_qcom gpio_keys qrtr [last unloaded: qcom_edac]
[ 8477.006395] CPU: 7 PID: 0 Comm: swapper/7 Tainted: G D
6.1.0-next-20221219 #25
[ 8477.006399] Hardware name: LENOVO 21BX0015US/21BX0015US, BIOS
N3HET74W (1.46 ) 10/12/2022
[ 8477.006401] pstate: 204003c5 (nzCv DAIF +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 8477.006405] pc : ct_kernel_exit.constprop.0+0xe0/0xf0
[ 8477.006411] lr : ct_kernel_exit.constprop.0+0x48/0xf0
[ 8477.006417] sp : ffff800008253d50
[ 8477.006419] x29: ffff800008253d50 x28: 0000000000000000 x27: 0000000000000000
[ 8477.006424] x26: 0000000000000000 x25: 0000000000000000 x24: 000007b5b4f2b289
[ 8477.006430] x23: 0000000000000000 x22: ffff1b133bd4deb0 x21: ffffaf4f138ce178
[ 8477.006435] x20: ffffaf4f130c6940 x19: ffffaf4f1339eeb0 x18: ffff1b0bc9725a0c
[ 8477.006440] x17: 3430303030303030 x16: ffff1b0bc9725b00 x15: ffff80000ca1b5d8
[ 8477.006446] x14: 0000ffffee58afff x13: ffffffffffffffff x12: ffffaf4f138c9bb0
[ 8477.006451] x11: 0000000000000001 x10: 0000000000000b80 x9 : ffffaf4f12806da8
[ 8477.006457] x8 : ffff1b0bc03a3be0 x7 : 0000000000000004 x6 : 00000013d6932dd5
[ 8477.006462] x5 : 00ffffffffffffff x4 : 0000000000000001 x3 : ffffaf4f1338f008
[ 8477.006467] x2 : ffff6bc4289af000 x1 : 4000000000000002 x0 : 4000000000000000
[ 8477.006473] Call trace:
[ 8477.006475] ct_kernel_exit.constprop.0+0xe0/0xf0
[ 8477.006481] ct_idle_enter+0x10/0x20
[ 8477.006487] cpuidle_enter_state+0x244/0x4e0
[ 8477.006493] cpuidle_enter+0x40/0x60
[ 8477.006498] do_idle+0x234/0x2c0
[ 8477.006504] cpu_startup_entry+0x2c/0x3c
[ 8477.006509] secondary_start_kernel+0x128/0x150
[ 8477.006514] __secondary_switched+0xb0/0xb4
[ 8477.006520] ---[ end trace 0000000000000000 ]---

I'm not entirely sure it's this driver, but removing the module can
reliably reproduce it.
--steev
Borislav Petkov Dec. 20, 2022, 11:55 p.m. UTC | #2
On Tue, Dec 20, 2022 at 03:22:07PM +0530, Manivannan Sadhasivam wrote:
> This is a genuine use-after-free bug that happens because the edac core frees
> the memory assigned to "llcc_driv_data" pointer that gets passed as "pvt_info".
> 
> Here, the LLCC driver is one creating the "qcom_llcc_edac" platform device and
> also allocating memory for "llcc_driv_data". But since during qcom_edac driver
> removal, we are just unregistering the driver and the platform device still
> stays around, the edac driver is not supposed to free any memory associated
> with the platform device.

If you mean

__edac_device_free_ctl_info()

it is very well supposed to free it as it allocates it in
edac_device_alloc_ctl_info().

If qcom_llcc_edac_probe() simply goes and assigns something of its own
to edev_ctl->pvt_info, then that driver gets to keep the pieces ofc.
Manivannan Sadhasivam Dec. 21, 2022, 5:55 a.m. UTC | #3
On Wed, Dec 21, 2022 at 12:55:03AM +0100, Borislav Petkov wrote:
> On Tue, Dec 20, 2022 at 03:22:07PM +0530, Manivannan Sadhasivam wrote:
> > This is a genuine use-after-free bug that happens because the edac core frees
> > the memory assigned to "llcc_driv_data" pointer that gets passed as "pvt_info".
> > 
> > Here, the LLCC driver is one creating the "qcom_llcc_edac" platform device and
> > also allocating memory for "llcc_driv_data". But since during qcom_edac driver
> > removal, we are just unregistering the driver and the platform device still
> > stays around, the edac driver is not supposed to free any memory associated
> > with the platform device.
> 
> If you mean
> 
> __edac_device_free_ctl_info()
> 
> it is very well supposed to free it as it allocates it in
> edac_device_alloc_ctl_info().
> 
> If qcom_llcc_edac_probe() simply goes and assigns something of its own
> to edev_ctl->pvt_info, then that driver gets to keep the pieces ofc.
> 

Right. It is the issue of the qcom driver from the start.

Thanks,
Mani

> -- 
> Regards/Gruss,
>     Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette
diff mbox series

Patch

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
index 109c9d2b684d..0510a5d510e7 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc8280xp.dtsi
@@ -1856,8 +1856,14 @@  opp-6 {
 
 		system-cache-controller@9200000 {
 			compatible = "qcom,sc8280xp-llcc";
-			reg = <0 0x09200000 0 0x58000>, <0 0x09600000 0 0x58000>;
-			reg-names = "llcc_base", "llcc_broadcast_base";
+			reg = <0 0x09200000 0 0x58000>, <0 0x09280000 0 0x58000>,
+			      <0 0x09300000 0 0x58000>, <0 0x09380000 0 0x58000>,
+			      <0 0x09400000 0 0x58000>, <0 0x09480000 0 0x58000>,
+			      <0 0x09500000 0 0x58000>, <0 0x09580000 0 0x58000>,
+			      <0 0x09600000 0 0x58000>;
+			reg-names = "llcc0_base", "llcc1_base", "llcc2_base",
+				    "llcc3_base", "llcc4_base", "llcc5_base",
+				    "llcc6_base", "llcc7_base",  "llcc_broadcast_base";
 			interrupts = <GIC_SPI 582 IRQ_TYPE_LEVEL_HIGH>;
 		};