linux-kernel - Re: [RFC PATCH] drm/msm/dp: move link

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAA8EJpq=5=L5RdVZRkf=e2wyjQufnSzEC+=19FjCCF9S6SSEJA@mail.gmail.com>
Date: Wed, 6 Mar 2024 21:52:48 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Abhinav Kumar <quic_abhinavk@...cinc.com>
Cc: freedreno@...ts.freedesktop.org, Rob Clark <robdclark@...il.com>, 
	Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>, 
	David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org, 
	swboyd@...omium.org, quic_jesszhan@...cinc.com, quic_parellan@...cinc.com, 
	quic_khsieh@...cinc.com, linux-arm-msm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] drm/msm/dp: move link_ready out of HPD event thread

On Wed, 6 Mar 2024 at 21:50, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>
> There are cases where the userspace might still send another
> frame after the HPD disconnect causing a modeset cycle after
> a disconnect. This messes the internal state machine of MSM DP driver
> and can lead to a crash as there can be an imbalance between
> bridge_disable() and bridge_enable().
>
> This was also previously reported on [1] for which [2] was posted
> and helped resolve the issue by rejecting commits if the DP is not
> in connected state.
>
> The change resolved the bug but there can also be another race condition.
> If hpd_event_thread does not pick up the EV_USER_NOTIFICATION and process it
> link_ready will also not be set to false allowing the frame to sneak in.
>
> Lets move setting link_ready outside of hpd_event_thread() processing to
> eliminate a window of race condition.
>
> [1] : https://gitlab.freedesktop.org/drm/msm/-/issues/17
> [2] : https://lore.kernel.org/all/1664408211-25314-1-git-send-email-quic_khsieh@quicinc.com/
>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
> ---
>  drivers/gpu/drm/msm/dp/dp_display.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 068d44eeaa07..e00092904ccc 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -345,8 +345,6 @@ static int dp_display_send_hpd_notification(struct dp_display_private *dp,
>                                                          dp->panel->downstream_ports);
>         }
>
> -       dp->dp_display.link_ready = hpd;
> -
>         drm_dbg_dp(dp->drm_dev, "type=%d hpd=%d\n",
>                         dp->dp_display.connector_type, hpd);
>         drm_bridge_hpd_notify(bridge, dp->dp_display.link_ready);
> @@ -399,6 +397,8 @@ static int dp_display_process_hpd_high(struct dp_display_private *dp)
>                 goto end;
>         }
>
> +       dp->dp_display.link_ready = true;

Do we need any kind of locking now?

> +
>         dp_add_event(dp, EV_USER_NOTIFICATION, true, 0);
>
>  end:
> @@ -466,6 +466,8 @@ static int dp_display_notify_disconnect(struct device *dev)
>  {
>         struct dp_display_private *dp = dev_get_dp_display_private(dev);
>
> +       dp->dp_display.link_ready = false;
> +
>         dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
>
>         return 0;
> @@ -487,6 +489,7 @@ static int dp_display_handle_port_status_changed(struct dp_display_private *dp)
>                 drm_dbg_dp(dp->drm_dev, "sink count is zero, nothing to do\n");
>                 if (dp->hpd_state != ST_DISCONNECTED) {
>                         dp->hpd_state = ST_DISCONNECT_PENDING;
> +                       dp->dp_display.link_ready = false;
>                         dp_add_event(dp, EV_USER_NOTIFICATION, false, 0);
>                 }
>         } else {
> --
> 2.34.1
>


-- 
With best wishes
Dmitry