Commit 48f05c3b authored by Marek Szyprowski's avatar Marek Szyprowski Committed by Luca Ceresoli
Browse files

drm/bridge: analogix_dp: Use devm_drm_bridge_alloc() API 



devm_drm_bridge_alloc() is the new API to be used for allocating
(and partially initializing) a private driver struct embedding
a struct drm_bridge.

Analogix DP driver somehow missed the automated conversion in commit
9c399719 ("drm: convert many bridge drivers from devm_kzalloc() to
devm_drm_bridge_alloc() API"), what causes the following warning:

------------[ cut here ]------------
WARNING: lib/refcount.c:25 at drm_bridge_attach+0x2c/0x248, CPU#1: kworker/u8:1/34
refcount_t: addition on 0; use-after-free.
Modules linked in:
CPU: 1 UID: 0 PID: 34 Comm: kworker/u8:1 Not tainted 6.16.0-rc3-next-20250627-dirty #15839 PREEMPT
Hardware name: Samsung Exynos (Flattened Device Tree)
Workqueue: events_unbound deferred_probe_work_func
Call trace:
 unwind_backtrace from show_stack+0x10/0x14
 show_stack from dump_stack_lvl+0x68/0x88
 dump_stack_lvl from __warn+0x94/0x1f0
 __warn from warn_slowpath_fmt+0x124/0x1bc
 warn_slowpath_fmt from drm_bridge_attach+0x2c/0x248
 drm_bridge_attach from analogix_dp_bind+0x70/0xc8
 analogix_dp_bind from exynos_dp_bind+0x58/0xc4
 exynos_dp_bind from component_bind_all+0x11c/0x27c
 component_bind_all from exynos_drm_bind+0xe8/0x198
 exynos_drm_bind from try_to_bring_up_aggregate_device+0x200/0x2d8
 try_to_bring_up_aggregate_device from __component_add+0xb0/0x170
 __component_add from exynos_dp_probe+0xc0/0x164
 exynos_dp_probe from platform_probe+0x5c/0xb8
 platform_probe from really_probe+0xe0/0x3d8
 really_probe from __driver_probe_device+0x9c/0x1e0
 __driver_probe_device from driver_probe_device+0x30/0xc0
 driver_probe_device from __device_attach_driver+0xa8/0x120
 __device_attach_driver from bus_for_each_drv+0x84/0xdc
 bus_for_each_drv from __device_attach+0xb0/0x20c
 __device_attach from bus_probe_device+0x8c/0x90
 bus_probe_device from deferred_probe_work_func+0x98/0xe0
 deferred_probe_work_func from process_one_work+0x24c/0x70c
 process_one_work from worker_thread+0x1b8/0x3bc
 worker_thread from kthread+0x13c/0x264
 kthread from ret_from_fork+0x14/0x28
...
---[ end trace 0000000000000000 ]---

Fix this by switching the driver to the new API.

Note the above warning only appears starting with commit a7748dd1
("drm/bridge: get/put the bridge reference in drm_bridge_add/remove()")
which is the first commmit having added a drm_bridge_get/put() pair and
thus exposing the incorrect initial refcount issue.

Signed-off-by: default avatarMarek Szyprowski <m.szyprowski@samsung.com>
Fixes: a7748dd1 ("drm/bridge: get/put the bridge reference in drm_bridge_add/remove()")
Reviewed-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
Link: https://lore.kernel.org/r/20250627165652.580798-1-m.szyprowski@samsung.com


[Luca: add Fixes tag and mention the reason in commit message]
Signed-off-by: default avatarLuca Ceresoli <luca.ceresoli@bootlin.com>
parent e33f256d

drivers/gpu/drm/bridge/analogix/analogix_dp_core.c

+11 −29
Original line number Diff line number Diff line
@@ -1041,7 +1041,7 @@ static int analogix_dp_bridge_attach(struct drm_bridge *bridge, 
				     struct drm_encoder *encoder,

				     enum drm_bridge_attach_flags flags)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);

	struct drm_connector *connector = NULL;

	int ret = 0;
@@ -1125,7 +1125,7 @@ struct drm_crtc *analogix_dp_get_new_crtc(struct analogix_dp_device *dp, 
static void analogix_dp_bridge_atomic_pre_enable(struct drm_bridge *bridge,

						 struct drm_atomic_state *old_state)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);

	struct drm_crtc *crtc;

	struct drm_crtc_state *old_crtc_state;
@@ -1180,7 +1180,7 @@ static int analogix_dp_set_bridge(struct analogix_dp_device *dp) 
static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge,

					     struct drm_atomic_state *old_state)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);

	struct drm_crtc *crtc;

	struct drm_crtc_state *old_crtc_state;

	int timeout_loop = 0;
@@ -1217,7 +1217,7 @@ static void analogix_dp_bridge_atomic_enable(struct drm_bridge *bridge, 


static void analogix_dp_bridge_disable(struct drm_bridge *bridge)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);



	if (dp->dpms_mode != DRM_MODE_DPMS_ON)

		return;
@@ -1240,7 +1240,7 @@ static void analogix_dp_bridge_disable(struct drm_bridge *bridge) 
static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge,

					      struct drm_atomic_state *old_state)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);

	struct drm_crtc *old_crtc, *new_crtc;

	struct drm_crtc_state *old_crtc_state = NULL;

	struct drm_crtc_state *new_crtc_state = NULL;
@@ -1278,7 +1278,7 @@ static void analogix_dp_bridge_atomic_disable(struct drm_bridge *bridge, 
static void analogix_dp_bridge_atomic_post_disable(struct drm_bridge *bridge,

						   struct drm_atomic_state *old_state)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);

	struct drm_crtc *crtc;

	struct drm_crtc_state *new_crtc_state;

	int ret;
@@ -1300,7 +1300,7 @@ static void analogix_dp_bridge_mode_set(struct drm_bridge *bridge, 
				const struct drm_display_mode *orig_mode,

				const struct drm_display_mode *mode)

{

	struct analogix_dp_device *dp = bridge->driver_private;

	struct analogix_dp_device *dp = to_dp(bridge);

	struct drm_display_info *display_info = &dp->connector.display_info;

	struct video_info *video = &dp->video_info;

	struct device_node *dp_node = dp->dev->of_node;
@@ -1385,25 +1385,6 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = { 
	.attach = analogix_dp_bridge_attach,

};



static int analogix_dp_create_bridge(struct drm_device *drm_dev,

				     struct analogix_dp_device *dp)

{

	struct drm_bridge *bridge;



	bridge = devm_kzalloc(drm_dev->dev, sizeof(*bridge), GFP_KERNEL);

	if (!bridge) {

		DRM_ERROR("failed to allocate for drm bridge\n");

		return -ENOMEM;

	}



	dp->bridge = bridge;



	bridge->driver_private = dp;

	bridge->funcs = &analogix_dp_bridge_funcs;



	return drm_bridge_attach(dp->encoder, bridge, NULL, 0);

}



static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)

{

	struct device_node *dp_node = dp->dev->of_node;
@@ -1491,7 +1472,8 @@ analogix_dp_probe(struct device *dev, struct analogix_dp_plat_data *plat_data) 
		return ERR_PTR(-EINVAL);

	}



	dp = devm_kzalloc(dev, sizeof(struct analogix_dp_device), GFP_KERNEL);

	dp = devm_drm_bridge_alloc(dev, struct analogix_dp_device, bridge,

				   &analogix_dp_bridge_funcs);

	if (!dp)

		return ERR_PTR(-ENOMEM);
@@ -1643,7 +1625,7 @@ int analogix_dp_bind(struct analogix_dp_device *dp, struct drm_device *drm_dev) 
		return ret;

	}



	ret = analogix_dp_create_bridge(drm_dev, dp);

	ret = drm_bridge_attach(dp->encoder, &dp->bridge, NULL, 0);

	if (ret) {

		DRM_ERROR("failed to create bridge (%d)\n", ret);

		goto err_unregister_aux;
@@ -1660,7 +1642,7 @@ EXPORT_SYMBOL_GPL(analogix_dp_bind); 


void analogix_dp_unbind(struct analogix_dp_device *dp)

{

	analogix_dp_bridge_disable(dp->bridge);

	analogix_dp_bridge_disable(&dp->bridge);

	dp->connector.funcs->destroy(&dp->connector);



	drm_panel_unprepare(dp->plat_data->panel);

drivers/gpu/drm/bridge/analogix/analogix_dp_core.h

+2 −1
Original line number Diff line number Diff line
@@ -11,6 +11,7 @@ 


#include <drm/display/drm_dp_helper.h>

#include <drm/drm_crtc.h>

#include <drm/drm_bridge.h>



#define DP_TIMEOUT_LOOP_COUNT 100

#define MAX_CR_LOOP 5
@@ -154,7 +155,7 @@ struct analogix_dp_device { 
	struct device		*dev;

	struct drm_device	*drm_dev;

	struct drm_connector	connector;

	struct drm_bridge	*bridge;

	struct drm_bridge	bridge;

	struct drm_dp_aux       aux;

	struct clk		*clock;

	unsigned int		irq;