linux-kernel - Re: [PATCH] drm/dp_mst: Fix NULL deref in drm_dp_get_one_sb

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] [day] [month] [year] [list]

Message-ID: <CAMavQK+tFSVx068FKnxD0X6TMWnf_TKPHy4ZmR=CD8kGtk5A_w@mail.gmail.com>
Date:   Mon, 6 Apr 2020 15:45:31 -0400
From:   Sean Paul <sean@...rly.run>
To:     Lyude Paul <lyude@...hat.com>
Cc:     dri-devel <dri-devel@...ts.freedesktop.org>,
        Wayne Lin <Wayne.Lin@....com>, Wayne Lin <waynelin@....com>,
        Sean Paul <seanpaul@...omium.org>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Maxime Ripard <mripard@...nel.org>,
        Thomas Zimmermann <tzimmermann@...e.de>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/dp_mst: Fix NULL deref in drm_dp_get_one_sb_msg()

On Mon, Apr 6, 2020 at 3:34 PM Lyude Paul <lyude@...hat.com> wrote:
>
> While we don't need this function to store an mstb anywhere for UP
> requests since we process them asynchronously, we do need to make sure
> that we don't try to write to **mstb for UP requests otherwise we'll
> cause a NULL pointer deref:
>
>     RIP: 0010:drm_dp_get_one_sb_msg+0x4b/0x460 [drm_kms_helper]
>     Call Trace:
>      ? vprintk_emit+0x16a/0x230
>      ? drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
>      drm_dp_mst_hpd_irq+0x133/0x1010 [drm_kms_helper]
>      ? __drm_dbg+0x87/0x90 [drm]
>      ? intel_dp_hpd_pulse+0x24b/0x400 [i915]
>      intel_dp_hpd_pulse+0x24b/0x400 [i915]
>      i915_digport_work_func+0xd6/0x160 [i915]
>      process_one_work+0x1a9/0x370
>      worker_thread+0x4d/0x3a0
>      kthread+0xf9/0x130
>      ? process_one_work+0x370/0x370
>      ? kthread_park+0x90/0x90
>      ret_from_fork+0x35/0x40
>
> So, fix this.

Ugggh, what a fail! I found this in Feb and posted the patch in
20200218171522.GF253734@..._vandelay. I had to migrate my workstation
due to WFH order and didn't apply the patch before pushing. Messy
messy messy.

Thanks for fixing!

Reviewed-by: Sean Paul <sean@...rly.run>

>
> Signed-off-by: Lyude Paul <lyude@...hat.com>
> Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies")
> Cc: Wayne Lin <Wayne.Lin@....com>
> Cc: Lyude Paul <lyude@...hat.com>
> Cc: Wayne Lin <waynelin@....com>
> Cc: Sean Paul <seanpaul@...omium.org>
> ---
>  drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 1ff49547b2e8..8751278b3941 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -3703,7 +3703,8 @@ static bool drm_dp_get_one_sb_msg(struct drm_dp_mst_topology_mgr *mgr, bool up,
>         int basereg = up ? DP_SIDEBAND_MSG_UP_REQ_BASE :
>                            DP_SIDEBAND_MSG_DOWN_REP_BASE;
>
> -       *mstb = NULL;
> +       if (!up)
> +               *mstb = NULL;
>         *seqno = -1;
>
>         len = min(mgr->max_dpcd_transaction_bytes, 16);
> --
> 2.25.1
>