Message-ID: <f5880400-ab7b-4cae-81e4-893ce34a0460@ideasonboard.com>
Date: Wed, 16 Apr 2025 15:26:17 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: Anusha Srivatsa <asrivats@...hat.com>,
 Paul Kocialkowski <paulk@...-base.io>, Dmitry Baryshkov <lumag@...nel.org>,
 Hervé Codina <herve.codina@...tlin.com>,
 Hui Pu <Hui.Pu@...ealthcare.com>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
 dri-devel@...ts.freedesktop.org, asahi@...ts.linux.dev,
 linux-kernel@...r.kernel.org, chrome-platform@...ts.linux.dev,
 imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 linux-mediatek@...ts.infradead.org, linux-amlogic@...ts.infradead.org,
 linux-renesas-soc@...r.kernel.org, platform-driver-x86@...r.kernel.org,
 linux-samsung-soc@...r.kernel.org, linux-arm-msm@...r.kernel.org,
 freedreno@...ts.freedesktop.org, linux-stm32@...md-mailman.stormreply.com,
 Michal Simek <michal.simek@....com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Andrzej Hajda <andrzej.hajda@...el.com>,
 Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
 Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
 Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
 Jagan Teki <jagan@...rulasolutions.com>, Shawn Guo <shawnguo@...nel.org>,
 Sascha Hauer <s.hauer@...gutronix.de>,
 Pengutronix Kernel Team <kernel@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, Douglas Anderson
 <dianders@...omium.org>, Chun-Kuang Hu <chunkuang.hu@...nel.org>,
 Krzysztof Kozlowski <krzk@...nel.org>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 "Sagar, Vishal" <vishal.sagar@....com>
Subject: Re: [PATCH 29/34] drm: zynqmp_dp: convert to devm_drm_bridge_alloc()
 API

Hi,

On 07/04/2025 17:23, Luca Ceresoli wrote:
> This is the new API for allocating DRM bridges.
> 
> This driver has a peculiar structure. zynqmp_dpsub.c is the actual driver,
> which delegates to a submodule (zynqmp_dp.c) the allocation of a
> sub-structure embedding the drm_bridge and its initialization, however it
> does not delegate the drm_bridge_add(). Hence, following carefully the code
> flow, it is correct to change the allocation function and .funcs assignment
> in the submodule, while the drm_bridge_add() is not in that submodule.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
> 
> ---
> 
> Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>
> Cc: Michal Simek <michal.simek@....com>
> Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
> ---
>   drivers/gpu/drm/xlnx/zynqmp_dp.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 11d2415fb5a1f7fad03421898331289f2295d68b..de22b6457a78a7a2110f9f308d0b5a8700544010 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -2439,9 +2439,9 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
>   	struct zynqmp_dp *dp;
>   	int ret;
>   
> -	dp = kzalloc(sizeof(*dp), GFP_KERNEL);
> -	if (!dp)
> -		return -ENOMEM;
> +	dp = devm_drm_bridge_alloc(&pdev->dev, struct zynqmp_dp, bridge, &zynqmp_dp_bridge_funcs);
> +	if (IS_ERR(dp))
> +		return PTR_ERR(dp);
>   
>   	dp->dev = &pdev->dev;
>   	dp->dpsub = dpsub;
> @@ -2488,7 +2488,6 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
>   
>   	/* Initialize the bridge. */
>   	bridge = &dp->bridge;
> -	bridge->funcs = &zynqmp_dp_bridge_funcs;
>   	bridge->ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
>   		    | DRM_BRIDGE_OP_HPD;
>   	bridge->type = DRM_MODE_CONNECTOR_DisplayPort;
> 

I haven't had time to look at this more, but jfyi: I got this when 
unloading modules, but it doesn't seem to happen every time:

[  103.010533] ------------[ cut here ]------------
[  103.015415] refcount_t: underflow; use-after-free.
[  103.020657] WARNING: CPU: 2 PID: 392 at lib/refcount.c:28 
refcount_warn_saturate+0xf4/0x148
[  103.029056] Modules linked in: zynqmp_dpsub(-) display_connector 
drm_display_helper drm_dma_helper drm_kms_helper drm drm_p
anel_orientation_quirks
[  103.042437] CPU: 2 UID: 0 PID: 392 Comm: rmmod Not tainted 
6.15.0-rc2+ #3 PREEMPT
[  103.050035] Hardware name: ZynqMP ZCU106 RevA (DT)
[  103.054836] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)
[  103.061814] pc : refcount_warn_saturate+0xf4/0x148
[  103.066632] lr : refcount_warn_saturate+0xf4/0x148
[  103.071441] sp : ffff800083b5bbb0
[  103.074766] x29: ffff800083b5bbb0 x28: ffff000806b23780 x27: 
0000000000000000
[  103.081953] x26: 0000000000000000 x25: 0000000000000000 x24: 
ffff000801a68400
[  103.089141] x23: ffff800081311a20 x22: ffff800083b5bc38 x21: 
ffff000801a68010
[  103.096329] x20: ffff0008040676c0 x19: ffff000804067240 x18: 
0000000000000006
[  103.103517] x17: 2e30303030303464 x16: 662d7968703a7968 x15: 
ffff800083b5b5a0
[  103.110705] x14: 0000000000000000 x13: 00000000000c0000 x12: 
0000000000000000
[  103.117892] x11: ffff80008163d6bc x10: 0000000000000028 x9 : 
ffff800080ead38c
[  103.125080] x8 : ffff800083b5b908 x7 : 0000000000000000 x6 : 
ffff800083b5b9c0
[  103.132268] x5 : ffff800083b5b948 x4 : 0000000000000001 x3 : 
00000000000000db
[  103.139455] x2 : 0000000000000000 x1 : 0000000000000000 x0 : 
ffff000806b23780
[  103.146644] Call trace:
[  103.149102]  refcount_warn_saturate+0xf4/0x148 (P)
[  103.153918]  drm_bridge_put.part.0+0x88/0xa0 [drm]
[  103.159188]  drm_bridge_put_void+0x1c/0x38 [drm]
[  103.164231]  devm_action_release+0x1c/0x30
[  103.168354]  release_nodes+0x68/0xa8
[  103.171957]  devres_release_all+0x98/0xf0
[  103.175993]  device_unbind_cleanup+0x20/0x70
[  103.180291]  device_release_driver_internal+0x208/0x250
[  103.185542]  driver_detach+0x54/0xa8
[  103.189145]  bus_remove_driver+0x78/0x108
[  103.193181]  driver_unregister+0x38/0x70
[  103.197131]  platform_driver_unregister+0x1c/0x30
[  103.201862]  zynqmp_dpsub_driver_exit+0x18/0x1100 [zynqmp_dpsub]
[  103.207931]  __arm64_sys_delete_module+0x1a8/0x2d0
[  103.212748]  invoke_syscall+0x50/0x120
[  103.216524]  el0_svc_common.constprop.0+0x48/0xf0
[  103.221256]  do_el0_svc+0x24/0x38
[  103.224598]  el0_svc+0x48/0x128
[  103.227766]  el0t_64_sync_handler+0x10c/0x138
[  103.232150]  el0t_64_sync+0x1a4/0x1a8
[  103.235841] irq event stamp: 7936
[  103.239173] hardirqs last  enabled at (7935): [<ffff8000800aaf78>] 
finish_task_switch.isra.0+0xb0/0x2a0
[  103.248600] hardirqs last disabled at (7936): [<ffff800080eaac74>] 
el1_dbg+0x24/0x90
[  103.256369] softirqs last  enabled at (7930): [<ffff800080066f98>] 
handle_softirqs+0x4a0/0x4c0
[  103.265007] softirqs last disabled at (7905): [<ffff800080010224>] 
__do_softirq+0x1c/0x28
[  103.273211] ---[ end trace 0000000000000000 ]---

  Tomi

Powered by blists - more mailing lists