Re: [PATCH] media: qcom: camss: fix VFE pm domain off

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024-11-28 10:10, Vladimir Zapolskiy wrote:
On 11/27/24 12:01, Yassine Oudjana wrote:
On 22/11/2024 5:06 am, Barnabás Czémán wrote:
Fix NULL pointer check before device_link_del
is called.

The intention is clear, but the context of the change is completely lost.

Fixes: eb73facec2c2 ("media: qcom: camss: Use common VFE pm_domain_on/pm_domain_off where applicable")

It's invalid, the change is not a fix.
I don't agree this patch is fixing NULL pointer dereference.

[ 92.989120] Unable to handle kernel NULL pointer dereference at virtual address 000000000000032c
[   92.989170] Mem abort info:
[   92.989186]   ESR = 0x0000000096000004
[   92.989203]   EC = 0x25: DABT (current EL), IL = 32 bits
[   92.989221]   SET = 0, FnV = 0
[   92.989237]   EA = 0, S1PTW = 0
[   92.989253]   FSC = 0x04: level 0 translation fault
[   92.989270] Data abort info:
[   92.989284]   ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
[   92.989300]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   92.989317]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   92.989335] user pgtable: 4k pages, 48-bit VAs, pgdp=00000001218a8000
[ 92.989354] [000000000000032c] pgd=0000000000000000, p4d=0000000000000000
[   92.989389] Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
[ 92.989408] Modules linked in: q6afe_dai q6asm_dai q6routing q6adm q6asm q6afe snd_q6dsp_common panel_lgphilips_sw43101 q6core venus_enc venus_dec videobuf2_dma_contig imx318 ak7375 snd_soc_wcd9335 regmap_slimbus snd_soc_wcd_classh apr snd_soc_apq8096 snd_soc_qcom_common snd_soc_core qcom_camss msm v4l2_fwnode snd_compress ath10k_pci v4l2_async ath10k_core snd_pcm nxp_nci_i2c drm_exec nxp_nci venus_core videobuf2_dma_sg snd_timer ath v4l2_mem2mem videobuf2_memops mac80211 drm_dp_aux_bus snd gpu_sched nci videobuf2_v4l2 libarc4 soundcore videodev nfc slim_qcom_ngd_ctrl drm_display_helper hci_uart pdr_interface videobuf2_common btqca drm_kms_helper slimbus i2c_qcom_cci bluetooth mc qcom_q6v5_pas qcom_q6v5_mss qcom_pil_info qcom_q6v5 qcom_sysmon qcom_common qmi_helpers mdt_loader socinfo rmtfs_mem pwm_ir_tx cfg80211 rfkill zram zsmalloc atmel_mxt_ts drm drm_panel_orientation_quirks dm_mod ip_tables [ 92.989981] CPU: 2 PID: 1365 Comm: pool-megapixels Not tainted 6.9.0-rc3+ #10
[   92.990003] Hardware name: Xiaomi Mi Note 2 (DT)
[ 92.990020] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   92.990042] pc : device_link_put_kref+0xc/0xb8
[   92.990071] lr : device_link_del+0x30/0x48
[   92.990089] sp : ffff80008a5db9d0
[ 92.990105] x29: ffff80008a5db9d0 x28: 0000000000000001 x27: 0000000000000000 [ 92.990143] x26: 0000000000000000 x25: ffff0000e79d9100 x24: ffff0000e79d9500 [ 92.990180] x23: ffff0000943f8568 x22: 00000000ffffffff x21: 0000000000000000 [ 92.990217] x20: 0000000000000000 x19: ffff800081352498 x18: 0000000000000000 [ 92.990253] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000168 [ 92.990288] x14: 0000000000000000 x13: 0000000000000191 x12: ffff800081259d58 [ 92.990324] x11: 0000000000000001 x10: 0000000000000a60 x9 : ffff80008a5db7e0 [ 92.990359] x8 : ffff0000e79d9bc0 x7 : 0000000000000004 x6 : 0000000000000190 [ 92.990396] x5 : 0000000000000057 x4 : 0000000000000000 x3 : 0000000000000000 [ 92.990430] x2 : ffff0000e79d9100 x1 : 0000000000000000 x0 : 0000000000000000
[   92.990466] Call trace:
[   92.990482]  device_link_put_kref+0xc/0xb8
[   92.990503]  device_link_del+0x30/0x48
[   92.990522]  vfe_pm_domain_off+0x24/0x38 [qcom_camss]
[   92.990566]  vfe_put+0x9c/0xd0 [qcom_camss]
[   92.990601]  vfe_set_power+0x48/0x58 [qcom_camss]
[   92.990636]  pipeline_pm_power_one+0x154/0x158 [videodev]
[   92.990683]  pipeline_pm_power+0x74/0xfc [videodev]
[   92.990720]  v4l2_pipeline_pm_use+0x54/0x90 [videodev]
[   92.990757]  v4l2_pipeline_pm_put+0x14/0x34 [videodev]
[   92.990793]  video_release+0x2c/0x44 [qcom_camss]
[   92.990828]  v4l2_release+0xe4/0xec [videodev]
[   92.990865]  __fput+0x78/0x2c4
[   92.990892]  ____fput+0x10/0x1c
[   92.990915]  task_work_run+0x74/0xcc
[   92.990936]  do_exit+0x2c4/0x8fc
[   92.990961]  do_group_exit+0x34/0x90
[   92.990984]  get_signal+0x870/0x880
[   92.991008]  do_signal+0x8c/0x12b8
[   92.991034]  do_notify_resume+0xc8/0x140
[   92.991054]  el0_svc+0xc4/0xd8
[   92.991079]  el0t_64_sync_handler+0x120/0x12c
[   92.991102]  el0t_64_sync+0x190/0x194
[   92.991127] Code: 17ffffda d503233f a9bf7bfd 910003fd (b9432c01)
[   92.991150] ---[ end trace 0000000000000000 ]---

Here is a similar example from camss:
static void camss_genpd_cleanup(struct camss *camss)
{
        if (camss->genpd_num == 1)
                return;

        camss_genpd_subdevice_cleanup(camss);

        if (camss->genpd_link)
                device_link_del(camss->genpd_link);

        dev_pm_domain_detach(camss->genpd, true);
}

Signed-off-by: Barnabás Czémán <barnabas.czeman@xxxxxxxxxxxxxx>
---
   drivers/media/platform/qcom/camss/camss-vfe.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)


---
base-commit: decc701f41d07481893fdea942c0ac6b226e84cd
change-id: 20241122-vfe_pm_domain_off-c57008e54167

Best regards,

diff --git a/drivers/media/platform/qcom/camss/camss-vfe.c b/drivers/media/platform/qcom/camss/camss-vfe.c index 80a62ba11295042802cbaec617fb87c492ea6a55..1bf1473331f63b9ab106d21ea263c84d851c8a31 100644
--- a/drivers/media/platform/qcom/camss/camss-vfe.c
+++ b/drivers/media/platform/qcom/camss/camss-vfe.c
@@ -595,7 +595,7 @@ void vfe_isr_reset_ack(struct vfe_device *vfe)
    */
   void vfe_pm_domain_off(struct vfe_device *vfe)
   {
-	if (!vfe->genpd)
+	if (!vfe->genpd_link)
   		return;
      	device_link_del(vfe->genpd_link);

Tested-by: Yassine Oudjana <y.oudjana@xxxxxxxxxxxxxx>


The change is useless and it shall be dropped.

--
Best wishes,
Vladimir




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux