| Message ID | 20200406193352.1245985-1-lyude@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/dp_mst: Fix NULL deref in drm_dp_get_one_sb_msg() | expand |
On Mon, Apr 6, 2020 at 3:34 PM Lyude Paul <lyude@redhat.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@art_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@poorly.run> > > Signed-off-by: Lyude Paul <lyude@redhat.com> > Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies") > Cc: Wayne Lin <Wayne.Lin@amd.com> > Cc: Lyude Paul <lyude@redhat.com> > Cc: Wayne Lin <waynelin@amd.com> > Cc: Sean Paul <seanpaul@chromium.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 >
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);
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. Signed-off-by: Lyude Paul <lyude@redhat.com> Fixes: fbc821c4a506 ("drm/mst: Support simultaneous down replies") Cc: Wayne Lin <Wayne.Lin@amd.com> Cc: Lyude Paul <lyude@redhat.com> Cc: Wayne Lin <waynelin@amd.com> Cc: Sean Paul <seanpaul@chromium.org> --- drivers/gpu/drm/drm_dp_mst_topology.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)