Skip to content

Commit

Permalink
drm/bridge: aux-hpd: separate allocation and registration
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Johan Hovold authored and Dmitry Baryshkov committed Feb 23, 2024
1 parent 9ee485b commit e5ca263
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 15 deletions.
67 changes: 52 additions & 15 deletions drivers/gpu/drm/bridge/aux-hpd-bridge.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,28 +30,23 @@ static void drm_aux_hpd_bridge_release(struct device *dev)
kfree(adev);
}

static void drm_aux_hpd_bridge_unregister_adev(void *_adev)
static void drm_aux_hpd_bridge_free_adev(void *_adev)
{
struct auxiliary_device *adev = _adev;

auxiliary_device_delete(adev);
auxiliary_device_uninit(adev);
auxiliary_device_uninit(_adev);
}

/**
* drm_dp_hpd_bridge_register - Create a simple HPD DisplayPort bridge
* devm_drm_dp_hpd_bridge_alloc - allocate a HPD DisplayPort bridge
* @parent: device instance providing this bridge
* @np: device node pointer corresponding to this bridge instance
*
* Creates a simple DRM bridge with the type set to
* DRM_MODE_CONNECTOR_DisplayPort, which terminates the bridge chain and is
* able to send the HPD events.
*
* Return: device instance that will handle created bridge or an error code
* encoded into the pointer.
* Return: bridge auxiliary device pointer or an error pointer
*/
struct device *drm_dp_hpd_bridge_register(struct device *parent,
struct device_node *np)
struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent, struct device_node *np)
{
struct auxiliary_device *adev;
int ret;
Expand Down Expand Up @@ -82,13 +77,55 @@ struct device *drm_dp_hpd_bridge_register(struct device *parent,
return ERR_PTR(ret);
}

ret = auxiliary_device_add(adev);
if (ret) {
auxiliary_device_uninit(adev);
ret = devm_add_action_or_reset(parent, drm_aux_hpd_bridge_free_adev, adev);
if (ret)
return ERR_PTR(ret);
}

ret = devm_add_action_or_reset(parent, drm_aux_hpd_bridge_unregister_adev, adev);
return adev;
}
EXPORT_SYMBOL_GPL(devm_drm_dp_hpd_bridge_alloc);

static void drm_aux_hpd_bridge_del_adev(void *_adev)
{
auxiliary_device_delete(_adev);
}

/**
* devm_drm_dp_hpd_bridge_add - register a HDP DisplayPort bridge
* @dev: struct device to tie registration lifetime to
* @adev: bridge auxiliary device to be registered
*
* Returns: zero on success or a negative errno
*/
int devm_drm_dp_hpd_bridge_add(struct device *dev, struct auxiliary_device *adev)
{
int ret;

ret = auxiliary_device_add(adev);
if (ret)
return ret;

return devm_add_action_or_reset(dev, drm_aux_hpd_bridge_del_adev, adev);
}
EXPORT_SYMBOL_GPL(devm_drm_dp_hpd_bridge_add);

/**
* drm_dp_hpd_bridge_register - allocate and register a HDP DisplayPort bridge
* @parent: device instance providing this bridge
* @np: device node pointer corresponding to this bridge instance
*
* Return: device instance that will handle created bridge or an error pointer
*/
struct device *drm_dp_hpd_bridge_register(struct device *parent, struct device_node *np)
{
struct auxiliary_device *adev;
int ret;

adev = devm_drm_dp_hpd_bridge_alloc(parent, np);
if (IS_ERR(adev))
return ERR_CAST(adev);

ret = devm_drm_dp_hpd_bridge_add(parent, adev);
if (ret)
return ERR_PTR(ret);

Expand Down
15 changes: 15 additions & 0 deletions include/drm/bridge/aux-bridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@

#include <drm/drm_connector.h>

struct auxiliary_device;

#if IS_ENABLED(CONFIG_DRM_AUX_BRIDGE)
int drm_aux_bridge_register(struct device *parent);
#else
Expand All @@ -19,10 +21,23 @@ static inline int drm_aux_bridge_register(struct device *parent)
#endif

#if IS_ENABLED(CONFIG_DRM_AUX_HPD_BRIDGE)
struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent, struct device_node *np);
int devm_drm_dp_hpd_bridge_add(struct device *dev, struct auxiliary_device *adev);
struct device *drm_dp_hpd_bridge_register(struct device *parent,
struct device_node *np);
void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status);
#else
static inline struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent,
struct device_node *np)
{
return NULL;
}

static inline int devm_drm_dp_hpd_bridge_add(struct auxiliary_device *adev)
{
return 0;
}

static inline struct device *drm_dp_hpd_bridge_register(struct device *parent,
struct device_node *np)
{
Expand Down

0 comments on commit e5ca263

Please sign in to comment.