Re: [PATCH 1/2] drm/exynos: fix null pointer dereference issue
2014-11-21 23:41 GMT+09:00 Gustavo Padovan gust...@padovan.org: 2014-11-21 Inki Dae inki@samsung.com: On 2014년 11월 21일 08:12, Gustavo Padovan wrote: 2014-11-13 Inki Dae inki@samsung.com: This patch fixes null pointer dereference issue incurred when ipp driver is enabled and Exynos drm driver is closed. Non kms driver should register its own sub driver to setup necessary resources, which is done by load(). So null pointer dereference occurs when ipp driver is enabled and Exynos drm driver is closed because ipp core device is registered after component_master_add_with_match call. This patch makes exynos_drm_device_subdrv_probe() to be called after all non kms drivers are registered. This patch is breaking exynos initialization, exynos_drm_device_subdrv_probe() needs the drvdata but it is still NULL at this point which make the whole exynos init fails. The drvdata is only set in exynos_drm_load() so we need call exynos_drm_device_subdrv_probe() after that. There might be my missing point but with this patch, exynos_drm_device_subdrv_probe() will be called after exynos_drm_load() call because all kms drivers are probed before component_master_add_with_match call so exynos_drm_load() must be called by component_master_add_with_match function before exynos_drm_device_subdrv_probe call. So could you show me the error messages you faced with? There might be a corner case I missed. I've added some debug output to it. It fails on exynos_drm_device_subdrv_probe() because the drvdata is NULL. I've added debug output to exynos_drm_load() and as you can see it doesn't get called before subdrv_probe(). I'm testing this on snow. [1.767835] [drm] Initialized drm 1.1.0 20060810 [1.771120] [drm:exynos_drm_init] [1.774774] [drm:exynos_drm_platform_probe] [1.778760] platform exynos-drm: Driver exynos-drm requests probe deferral [1.786178] platform 145b.dp-controller: Driver exynos-dp requests probe deferral [1.794374] exynos-drm-ipp exynos-drm-ipp: drm ipp registered successfully. [1.800372] [drm:exynos_drm_device_subdrv_probe] dev (null) Still strange to me. I'd like to reproduce this error because it's impossible that exynos_drm_device_subdrv_probe is called without load call. even If error, this patch must be no problem so in this case, we would need check if there might be something we missed about component binding not this patch. So can you let me know where the git repository is and which defconfig file did you tested? Thanks, Inki Dae Gustavo ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] drm/exynos: fix null pointer dereference issue
2014-11-23 0:28 GMT+09:00 Inki Dae inki@samsung.com: 2014-11-21 23:41 GMT+09:00 Gustavo Padovan gust...@padovan.org: 2014-11-21 Inki Dae inki@samsung.com: On 2014년 11월 21일 08:12, Gustavo Padovan wrote: 2014-11-13 Inki Dae inki@samsung.com: This patch fixes null pointer dereference issue incurred when ipp driver is enabled and Exynos drm driver is closed. Non kms driver should register its own sub driver to setup necessary resources, which is done by load(). So null pointer dereference occurs when ipp driver is enabled and Exynos drm driver is closed because ipp core device is registered after component_master_add_with_match call. This patch makes exynos_drm_device_subdrv_probe() to be called after all non kms drivers are registered. This patch is breaking exynos initialization, exynos_drm_device_subdrv_probe() needs the drvdata but it is still NULL at this point which make the whole exynos init fails. The drvdata is only set in exynos_drm_load() so we need call exynos_drm_device_subdrv_probe() after that. There might be my missing point but with this patch, exynos_drm_device_subdrv_probe() will be called after exynos_drm_load() call because all kms drivers are probed before component_master_add_with_match call so exynos_drm_load() must be called by component_master_add_with_match function before exynos_drm_device_subdrv_probe call. So could you show me the error messages you faced with? There might be a corner case I missed. I've added some debug output to it. It fails on exynos_drm_device_subdrv_probe() because the drvdata is NULL. I've added debug output to exynos_drm_load() and as you can see it doesn't get called before subdrv_probe(). I'm testing this on snow. [1.767835] [drm] Initialized drm 1.1.0 20060810 [1.771120] [drm:exynos_drm_init] [1.774774] [drm:exynos_drm_platform_probe] [1.778760] platform exynos-drm: Driver exynos-drm requests probe deferral [1.786178] platform 145b.dp-controller: Driver exynos-dp requests probe deferral [1.794374] exynos-drm-ipp exynos-drm-ipp: drm ipp registered successfully. [1.800372] [drm:exynos_drm_device_subdrv_probe] dev (null) Still strange to me. I'd like to reproduce this error because it's impossible that exynos_drm_device_subdrv_probe is called without load call. even If error, this patch must be no problem so in this case, we would need check if there might be something we missed about component binding not this patch. But it's better to call exynos_drm_device_subdrv_probe() in load() like before because we should move all register codes of sub drivers into init() for the meantime. So let's just revert it. Thanks, Inki Dae So can you let me know where the git repository is and which defconfig file did you tested? Thanks, Inki Dae Gustavo ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] drm/exynos: fix null pointer dereference issue
2014-11-21 Inki Dae inki@samsung.com: On 2014년 11월 21일 08:12, Gustavo Padovan wrote: 2014-11-13 Inki Dae inki@samsung.com: This patch fixes null pointer dereference issue incurred when ipp driver is enabled and Exynos drm driver is closed. Non kms driver should register its own sub driver to setup necessary resources, which is done by load(). So null pointer dereference occurs when ipp driver is enabled and Exynos drm driver is closed because ipp core device is registered after component_master_add_with_match call. This patch makes exynos_drm_device_subdrv_probe() to be called after all non kms drivers are registered. This patch is breaking exynos initialization, exynos_drm_device_subdrv_probe() needs the drvdata but it is still NULL at this point which make the whole exynos init fails. The drvdata is only set in exynos_drm_load() so we need call exynos_drm_device_subdrv_probe() after that. There might be my missing point but with this patch, exynos_drm_device_subdrv_probe() will be called after exynos_drm_load() call because all kms drivers are probed before component_master_add_with_match call so exynos_drm_load() must be called by component_master_add_with_match function before exynos_drm_device_subdrv_probe call. So could you show me the error messages you faced with? There might be a corner case I missed. I've added some debug output to it. It fails on exynos_drm_device_subdrv_probe() because the drvdata is NULL. I've added debug output to exynos_drm_load() and as you can see it doesn't get called before subdrv_probe(). I'm testing this on snow. [1.767835] [drm] Initialized drm 1.1.0 20060810 [1.771120] [drm:exynos_drm_init] [1.774774] [drm:exynos_drm_platform_probe] [1.778760] platform exynos-drm: Driver exynos-drm requests probe deferral [1.786178] platform 145b.dp-controller: Driver exynos-dp requests probe deferral [1.794374] exynos-drm-ipp exynos-drm-ipp: drm ipp registered successfully. [1.800372] [drm:exynos_drm_device_subdrv_probe] dev (null) Gustavo -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] drm/exynos: fix null pointer dereference issue
2014-11-13 Inki Dae inki@samsung.com: This patch fixes null pointer dereference issue incurred when ipp driver is enabled and Exynos drm driver is closed. Non kms driver should register its own sub driver to setup necessary resources, which is done by load(). So null pointer dereference occurs when ipp driver is enabled and Exynos drm driver is closed because ipp core device is registered after component_master_add_with_match call. This patch makes exynos_drm_device_subdrv_probe() to be called after all non kms drivers are registered. This patch is breaking exynos initialization, exynos_drm_device_subdrv_probe() needs the drvdata but it is still NULL at this point which make the whole exynos init fails. The drvdata is only set in exynos_drm_load() so we need call exynos_drm_device_subdrv_probe() after that. Do you have the crash output for this? What is the issue you are fixing? Usually you should add this kind of information to you commit message, it helps us understand what you are fixing, specially in cases when a regression is introduced, like this patch for example Gustavo -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] drm/exynos: fix null pointer dereference issue
On 2014년 11월 21일 08:12, Gustavo Padovan wrote: 2014-11-13 Inki Dae inki@samsung.com: This patch fixes null pointer dereference issue incurred when ipp driver is enabled and Exynos drm driver is closed. Non kms driver should register its own sub driver to setup necessary resources, which is done by load(). So null pointer dereference occurs when ipp driver is enabled and Exynos drm driver is closed because ipp core device is registered after component_master_add_with_match call. This patch makes exynos_drm_device_subdrv_probe() to be called after all non kms drivers are registered. This patch is breaking exynos initialization, exynos_drm_device_subdrv_probe() needs the drvdata but it is still NULL at this point which make the whole exynos init fails. The drvdata is only set in exynos_drm_load() so we need call exynos_drm_device_subdrv_probe() after that. There might be my missing point but with this patch, exynos_drm_device_subdrv_probe() will be called after exynos_drm_load() call because all kms drivers are probed before component_master_add_with_match call so exynos_drm_load() must be called by component_master_add_with_match function before exynos_drm_device_subdrv_probe call. So could you show me the error messages you faced with? There might be a corner case I missed. Do you have the crash output for this? What is the issue you are fixing? Ok, below is the error messages, # modetest [5.653291] [ cut here ] [5.656469] WARNING: CPU: 2 PID: 1404 at kernel/locking/mutex.c:511 __mutex_lock_slowpath+0x3d4/0x3d8() [5.665816] DEBUG_LOCKS_WARN_ON(l-magic != l) [5.670069] Modules linked in: [5.673286] CPU: 2 PID: 1404 Comm: modetest Not tainted 3.18.0-rc3-146775-gbcfef97 #1149 [5.681389] [c0014400] (unwind_backtrace) from [c0011570] (show_stack+0x10/0x14) [5.689090] [c0011570] (show_stack) from [c0474060] (dump_stack+0x84/0xc4) [5.696304] [c0474060] (dump_stack) from [c0021918] (warn_slowpath_common+0x6c/0x88) [5.704364] [c0021918] (warn_slowpath_common) from [c0021964] (warn_slowpath_fmt+0x30/0x40) [5.713047] [c0021964] (warn_slowpath_fmt) from [c0477a4c] (__mutex_lock_slowpath+0x3d4/0x3d8) [5.721984] [c0477a4c] (__mutex_lock_slowpath) from [c0477a5c] (mutex_lock+0xc/0x24) [5.730069] [c0477a5c] (mutex_lock) from [c028e6fc] (ipp_subdrv_close+0x4c/0x13c) [5.737881] [c028e6fc] (ipp_subdrv_close) from [c027a51c] (exynos_drm_subdrv_close+0x3c/0x4c) [5.746731] [c027a51c] (exynos_drm_subdrv_close) from [c025eadc] (drm_release+0x94/0x4c8) [5.755228] [c025eadc] (drm_release) from [c00cbdd4] (__fput+0x80/0x1c8) [5.762268] [c00cbdd4] (__fput) from [c0037840] (task_work_run+0xac/0xe4) [5.769382] [c0037840] (task_work_run) from [c00110f8] (do_work_pending+0x94/0xb4) [5.777275] [c00110f8] (do_work_pending) from [c000e6e0] (work_pending+0xc/0x20) [5.784994] ---[ end trace bb48a41ae89d1f25 ]--- [5.789598] Unable to handle kernel NULL pointer dereference at virtual address [5.797664] pgd = ee3b8000 [5.800354] [] *pgd=6e366831, *pte=, *ppte= [5.806610] Internal error: Oops: 817 [#1] PREEMPT SMP ARM [5.812074] Modules linked in: [5.815117] CPU: 2 PID: 1404 Comm: modetest Tainted: GW 3.18.0-rc3-146775-gbcfef97 #1149 [5.824314] task: eea90800 ti: ee33c000 task.ti: ee33c000 [5.829704] PC is at __mutex_lock_slowpath+0xf4/0x3d8 [5.834730] LR is at __mutex_lock_slowpath+0xdc/0x3d8 [5.839765] pc : [c047776c]lr : [c0477754]psr: 8093 [5.839765] sp : ee33de88 ip : ee33de98 fp : c06cb814 [5.851220] r10: ee0f5854 r9 : c0700784 r8 : eea90800 [5.856429] r7 : ee33c008 r6 : 6013 r5 : ee0f5844 r4 : ee0f5840 [5.862938] r3 : r2 : r1 : ee33de88 r0 : ee0f5840 [5.869451] Flags: Nzcv IRQs off FIQs on Mode SVC_32 ISA ARM Segment user [5.876654] Control: 10c5387d Table: 6e3b804a DAC: 0015 [5.882383] Process modetest (pid: 1404, stack limit = 0xee33c240) [5.888544] Stack: (0xee33de88 to 0xee33e000) [5.892888] de80: ee0f5854 ee33de88 0001d030 ee0f5840 [5.901048] dea0: c06b1d94 ee0f5810 c0705e4c ee0eeb00 ee0f5840 ee0f5810 c0477a5c [5.909207] dec0: eeb4a510 c028e6fc ee14a434 eeb4a510 c06b2fe4 ee14a400 ee33c000 eeb4a510 [5.917366] dee0: c06b1d94 ee0eeb00 ee14a400 ee14a434 0008 ee0eea08 c027a51c [5.925525] df00: c0705e4c ee0eeb00 ee0eea00 ee14a400 ee14a434 c025eadc ee0eea08 0001 [5.933684] df20: eebce000 0021 ee0eea00 ee3354e0 [5.941843] df40: ee32a250 ee711428 0008 ee0eea08 c00cbdd4 [5.950002] df60: eea90b4c c06ca604 eea90800 c000e824 ee33c000 c0037840 [5.958161] df80: ee33c018 c000e824 ee33dfb0 ee33c000 c000e824 c00110f8 0003 0001 [5.966320] dfa0: beff0a4c 0006
[PATCH 1/2] drm/exynos: fix null pointer dereference issue
This patch fixes null pointer dereference issue incurred
when ipp driver is enabled and Exynos drm driver is closed.
Non kms driver should register its own sub driver to setup necessary
resources, which is done by load(). So null pointer dereference
occurs when ipp driver is enabled and Exynos drm driver is closed
because ipp core device is registered after component_master_add_with_match
call.
This patch makes exynos_drm_device_subdrv_probe() to be called after all non
kms drivers are registered.
Signed-off-by: Inki Dae inki@samsung.com
---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c
b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index e5c4c6c..f0bcdc6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -108,11 +108,6 @@ static int exynos_drm_load(struct drm_device *dev,
unsigned long flags)
if (ret)
goto err_unbind_all;
- /* Probe non kms sub drivers and virtual display driver. */
- ret = exynos_drm_device_subdrv_probe(dev);
- if (ret)
- goto err_cleanup_vblank;
-
/*
* enable drm irq mode.
* - with irq_enabled = true, we can use the vblank feature.
@@ -138,8 +133,6 @@ static int exynos_drm_load(struct drm_device *dev, unsigned
long flags)
return 0;
-err_cleanup_vblank:
- drm_vblank_cleanup(dev);
err_unbind_all:
component_unbind_all(dev-dev, dev);
err_mode_config_cleanup:
@@ -153,8 +146,6 @@ err_free_private:
static int exynos_drm_unload(struct drm_device *dev)
{
- exynos_drm_device_subdrv_remove(dev);
-
exynos_drm_fbdev_fini(dev);
drm_kms_helper_poll_fini(dev);
@@ -636,9 +627,16 @@ static int exynos_drm_platform_probe(struct
platform_device *pdev)
goto err_unregister_ipp_drv;
#endif
+ /* Probe non kms sub drivers and virtual display driver. */
+ ret = exynos_drm_device_subdrv_probe(platform_get_drvdata(pdev));
+ if (ret)
+ goto err_unregister_resources;
+
return ret;
+err_unregister_resources:
#ifdef CONFIG_DRM_EXYNOS_IPP
+ exynos_platform_device_ipp_unregister();
err_unregister_ipp_drv:
platform_driver_unregister(ipp_driver);
err_unregister_gsc_drv:
@@ -691,6 +689,8 @@ err_unregister_fimd_drv:
static int exynos_drm_platform_remove(struct platform_device *pdev)
{
+ exynos_drm_device_subdrv_remove(platform_get_drvdata(pdev));
+
#ifdef CONFIG_DRM_EXYNOS_IPP
exynos_platform_device_ipp_unregister();
platform_driver_unregister(ipp_driver);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
