Skip to content

Commit e5ca263

Browse files
jhovoldlumag
authored andcommitted
drm/bridge: aux-hpd: separate allocation and registration
Combining allocation and registration is an anti-pattern that should be avoided. Add two new functions for allocating and registering an dp-hpd bridge with a proper 'devm' prefix so that it is clear that these are device managed interfaces. devm_drm_dp_hpd_bridge_alloc() devm_drm_dp_hpd_bridge_add() The new interface will be used to fix a use-after-free bug in the Qualcomm PMIC GLINK driver and may prevent similar issues from being introduced elsewhere. The existing drm_dp_hpd_bridge_register() is reimplemented using the above and left in place for now. Signed-off-by: Johan Hovold <johan+linaro@kernel.org> Reviewed-by: Bjorn Andersson <andersson@kernel.org> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Link: https://patchwork.freedesktop.org/patch/msgid/20240217150228.5788-3-johan+linaro@kernel.org
1 parent 9ee485b commit e5ca263

2 files changed

Lines changed: 67 additions & 15 deletions

File tree

drivers/gpu/drm/bridge/aux-hpd-bridge.c

Lines changed: 52 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,28 +30,23 @@ static void drm_aux_hpd_bridge_release(struct device *dev)
3030
kfree(adev);
3131
}
3232

33-
static void drm_aux_hpd_bridge_unregister_adev(void *_adev)
33+
static void drm_aux_hpd_bridge_free_adev(void *_adev)
3434
{
35-
struct auxiliary_device *adev = _adev;
36-
37-
auxiliary_device_delete(adev);
38-
auxiliary_device_uninit(adev);
35+
auxiliary_device_uninit(_adev);
3936
}
4037

4138
/**
42-
* drm_dp_hpd_bridge_register - Create a simple HPD DisplayPort bridge
39+
* devm_drm_dp_hpd_bridge_alloc - allocate a HPD DisplayPort bridge
4340
* @parent: device instance providing this bridge
4441
* @np: device node pointer corresponding to this bridge instance
4542
*
4643
* Creates a simple DRM bridge with the type set to
4744
* DRM_MODE_CONNECTOR_DisplayPort, which terminates the bridge chain and is
4845
* able to send the HPD events.
4946
*
50-
* Return: device instance that will handle created bridge or an error code
51-
* encoded into the pointer.
47+
* Return: bridge auxiliary device pointer or an error pointer
5248
*/
53-
struct device *drm_dp_hpd_bridge_register(struct device *parent,
54-
struct device_node *np)
49+
struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent, struct device_node *np)
5550
{
5651
struct auxiliary_device *adev;
5752
int ret;
@@ -82,13 +77,55 @@ struct device *drm_dp_hpd_bridge_register(struct device *parent,
8277
return ERR_PTR(ret);
8378
}
8479

85-
ret = auxiliary_device_add(adev);
86-
if (ret) {
87-
auxiliary_device_uninit(adev);
80+
ret = devm_add_action_or_reset(parent, drm_aux_hpd_bridge_free_adev, adev);
81+
if (ret)
8882
return ERR_PTR(ret);
89-
}
9083

91-
ret = devm_add_action_or_reset(parent, drm_aux_hpd_bridge_unregister_adev, adev);
84+
return adev;
85+
}
86+
EXPORT_SYMBOL_GPL(devm_drm_dp_hpd_bridge_alloc);
87+
88+
static void drm_aux_hpd_bridge_del_adev(void *_adev)
89+
{
90+
auxiliary_device_delete(_adev);
91+
}
92+
93+
/**
94+
* devm_drm_dp_hpd_bridge_add - register a HDP DisplayPort bridge
95+
* @dev: struct device to tie registration lifetime to
96+
* @adev: bridge auxiliary device to be registered
97+
*
98+
* Returns: zero on success or a negative errno
99+
*/
100+
int devm_drm_dp_hpd_bridge_add(struct device *dev, struct auxiliary_device *adev)
101+
{
102+
int ret;
103+
104+
ret = auxiliary_device_add(adev);
105+
if (ret)
106+
return ret;
107+
108+
return devm_add_action_or_reset(dev, drm_aux_hpd_bridge_del_adev, adev);
109+
}
110+
EXPORT_SYMBOL_GPL(devm_drm_dp_hpd_bridge_add);
111+
112+
/**
113+
* drm_dp_hpd_bridge_register - allocate and register a HDP DisplayPort bridge
114+
* @parent: device instance providing this bridge
115+
* @np: device node pointer corresponding to this bridge instance
116+
*
117+
* Return: device instance that will handle created bridge or an error pointer
118+
*/
119+
struct device *drm_dp_hpd_bridge_register(struct device *parent, struct device_node *np)
120+
{
121+
struct auxiliary_device *adev;
122+
int ret;
123+
124+
adev = devm_drm_dp_hpd_bridge_alloc(parent, np);
125+
if (IS_ERR(adev))
126+
return ERR_CAST(adev);
127+
128+
ret = devm_drm_dp_hpd_bridge_add(parent, adev);
92129
if (ret)
93130
return ERR_PTR(ret);
94131

include/drm/bridge/aux-bridge.h

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99

1010
#include <drm/drm_connector.h>
1111

12+
struct auxiliary_device;
13+
1214
#if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE)
1315
int drm_aux_bridge_register(struct device *parent);
1416
#else
@@ -19,10 +21,23 @@ static inline int drm_aux_bridge_register(struct device *parent)
1921
#endif
2022

2123
#if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE)
24+
struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent, struct device_node *np);
25+
int devm_drm_dp_hpd_bridge_add(struct device *dev, struct auxiliary_device *adev);
2226
struct device *drm_dp_hpd_bridge_register(struct device *parent,
2327
struct device_node *np);
2428
void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status);
2529
#else
30+
static inline struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent,
31+
struct device_node *np)
32+
{
33+
return NULL;
34+
}
35+
36+
static inline int devm_drm_dp_hpd_bridge_add(struct auxiliary_device *adev)
37+
{
38+
return 0;
39+
}
40+
2641
static inline struct device *drm_dp_hpd_bridge_register(struct device *parent,
2742
struct device_node *np)
2843
{

0 commit comments

Comments
 (0)