| Message ID | 20220311093406.4068019-2-ricardo.canuelo@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Mitigate race condition problems when unbinding DRM driver | expand |
Il 11/03/22 10:34, Ricardo Cañuelo ha scritto: > When unbinding a DRM master driver there's a race condition that > sometimes results in an invalid vm access when userspace (gnome-shell) > issues a DRM_IOCTL_MODE_GETCONNECTOR ioctl right after a bridge has been > removed from an encoder's bridge chain. > > This means that once a bridge has been disabled and gone through > ps8640_post_disable(), if ps8640_bridge_get_edid() runs afterwards as a > result of that ioctl call it will call drm_bridge_chain_pre_enable() > and drm_bridge_chain_post_disable() again in a bridge that has been > already detached. > > Setting `ps_bridge->pre_enabled = false` at a later stage of the > bringdown path and calling drm_bridge_chain_pre_enable() inside > ps8640_bridge_get_edid() only if needed avoid this, although a proper > subsystem-wide fix would be the proper solution, since the same type of > race conditions might happen somewhere else. > > Tested on an Acer Chromebook R13 (Elm, MT8173) with Debian Sid. > > Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > drivers/gpu/drm/bridge/parade-ps8640.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) >
diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/bridge/parade-ps8640.c index 3f17337ee389..a927787a89bf 100644 --- a/drivers/gpu/drm/bridge/parade-ps8640.c +++ b/drivers/gpu/drm/bridge/parade-ps8640.c @@ -434,8 +434,6 @@ static void ps8640_post_disable(struct drm_bridge *bridge) { struct ps8640 *ps_bridge = bridge_to_ps8640(bridge); - ps_bridge->pre_enabled = false; - ps8640_bridge_vdo_control(ps_bridge, DISABLE); pm_runtime_put_sync_suspend(&ps_bridge->page[PAGE0_DP_CNTL]->dev); } @@ -487,6 +485,7 @@ static void ps8640_bridge_detach(struct drm_bridge *bridge) drm_dp_aux_unregister(&ps_bridge->aux); if (ps_bridge->link) device_link_del(ps_bridge->link); + ps_bridge->pre_enabled = false; } static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge, @@ -508,7 +507,8 @@ static struct edid *ps8640_bridge_get_edid(struct drm_bridge *bridge, * EDID, for this chip, we need to do a full poweron, otherwise it will * fail. */ - drm_bridge_chain_pre_enable(bridge); + if (poweroff) + drm_bridge_chain_pre_enable(bridge); edid = drm_get_edid(connector, ps_bridge->page[PAGE0_DP_CNTL]->adapter);
When unbinding a DRM master driver there's a race condition that sometimes results in an invalid vm access when userspace (gnome-shell) issues a DRM_IOCTL_MODE_GETCONNECTOR ioctl right after a bridge has been removed from an encoder's bridge chain. This means that once a bridge has been disabled and gone through ps8640_post_disable(), if ps8640_bridge_get_edid() runs afterwards as a result of that ioctl call it will call drm_bridge_chain_pre_enable() and drm_bridge_chain_post_disable() again in a bridge that has been already detached. Setting `ps_bridge->pre_enabled = false` at a later stage of the bringdown path and calling drm_bridge_chain_pre_enable() inside ps8640_bridge_get_edid() only if needed avoid this, although a proper subsystem-wide fix would be the proper solution, since the same type of race conditions might happen somewhere else. Tested on an Acer Chromebook R13 (Elm, MT8173) with Debian Sid. Signed-off-by: Ricardo Cañuelo <ricardo.canuelo@collabora.com> --- drivers/gpu/drm/bridge/parade-ps8640.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)