[Intel-gfx] [PATCH 1/4] drm/dp_mst: Fix NULL dereference during payload addition

[Lyude Paul]

Thanks for catching all of this! for the whole series:

Reviewed-by: Lyude Paul <lyude at redhat.com>


On Thu, 2023-09-14 at 01:32 +0300, Imre Deak wrote:
> Fix the NULL dereference leading to the following stack trace:
> 
> [  129.687181] i915 0000:00:02.0: [drm:drm_dp_add_payload_part1 [drm_display_helper]] VCPI 1 for port 000000005be4423e not in topology, not creating a payload to remote
> [  129.687257] BUG: kernel NULL pointer dereference, address: 0000000000000560
> [  129.694276] #PF: supervisor read access in kernel mode
> [  129.699459] #PF: error_code(0x0000) - not-present page
> [  129.704612] PGD 0 P4D 0
> [  129.707178] Oops: 0000 [#1] PREEMPT SMP NOPTI
> [  129.711556] CPU: 2 PID: 1623 Comm: Xorg Tainted: G     U             6.6.0-rc1-imre+ #985
> [  129.719744] Hardware name: Intel Corporation Alder Lake Client Platform/AlderLake-P DDR5 RVP, BIOS RPLPFWI1.R00.4035.A00.2301200723 01/20/2023
> [  129.732509] RIP: 0010:drm_dp_mst_topology_put_port+0x19/0x170 [drm_display_helper]
> [  129.740111] Code: 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41 55 41 54 53 48 89 fb 48 83 ec 08 <48> 8b 87 60 05 00 00 44 8b 0f 48 8b 70 58 41 83 e9 01 48 85 f6 74
> [  129.758842] RSP: 0018:ffffc90001daf900 EFLAGS: 00010286
> [  129.764104] RAX: 0000000000000001 RBX: 0000000000000000 RCX: 0000000000000000
> [  129.771246] RDX: 0000000000000000 RSI: ffffffff9e73d613 RDI: 0000000000000000
> [  129.778394] RBP: ffffc90001daf930 R08: 0000000000000000 R09: 0000000000000020
> [  129.785533] R10: 0000000000ffff10 R11: 000000000000000f R12: ffff888116c65e40
> [  129.792680] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> [  129.799822] FS:  00007f39f74b1a80(0000) GS:ffff88840f680000(0000) knlGS:0000000000000000
> [  129.807913] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [  129.813670] CR2: 0000000000000560 CR3: 0000000138b88000 CR4: 0000000000750ee0
> [  129.820815] PKRU: 55555554
> [  129.823551] Call Trace:
> [  129.826022]  <TASK>
> [  129.828154]  ? show_regs+0x65/0x70
> [  129.831599]  ? __die+0x24/0x70
> [  129.834683]  ? page_fault_oops+0x160/0x480
> [  129.838824]  ? dev_printk_emit+0x83/0xb0
> [  129.842797]  ? do_user_addr_fault+0x2e2/0x680
> [  129.847175]  ? exc_page_fault+0x78/0x180
> [  129.851143]  ? asm_exc_page_fault+0x27/0x30
> [  129.855353]  ? drm_dp_mst_topology_put_port+0x19/0x170 [drm_display_helper]
> [  129.862354]  drm_dp_add_payload_part1+0x85/0x100 [drm_display_helper]
> [  129.868832]  intel_mst_pre_enable_dp+0x1ef/0x240 [i915]
> [  129.874170]  intel_encoders_pre_enable+0x83/0xa0 [i915]
> [  129.879524]  hsw_crtc_enable+0xbe/0x750 [i915]
> [  129.884095]  intel_enable_crtc+0x68/0xa0 [i915]
> [  129.888752]  skl_commit_modeset_enables+0x2c4/0x5d0 [i915]
> [  129.894365]  intel_atomic_commit_tail+0x765/0x1070 [i915]
> [  129.899885]  intel_atomic_commit+0x3ba/0x400 [i915]
> [  129.904892]  drm_atomic_commit+0x96/0xd0 [drm]
> [  129.909405]  ? __pfx___drm_printfn_info+0x10/0x10 [drm]
> [  129.914698]  drm_atomic_helper_set_config+0x7e/0xc0 [drm_kms_helper]
> [  129.921102]  drm_mode_setcrtc+0x5af/0x8d0 [drm]
> [  129.925695]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
> [  129.930810]  drm_ioctl_kernel+0xc4/0x170 [drm]
> [  129.935317]  drm_ioctl+0x2a4/0x520 [drm]
> [  129.939305]  ? __pfx_drm_mode_setcrtc+0x10/0x10 [drm]
> [  129.944415]  ? __fget_light+0xa5/0x110
> [  129.948212]  __x64_sys_ioctl+0x98/0xd0
> [  129.951985]  do_syscall_64+0x37/0x90
> [  129.955581]  entry_SYSCALL_64_after_hwframe+0x6e/0xd8
> 
> Fixes: 5aa1dfcdf0a4 ("drm/mst: Refactor the flow for payload allocation/removement")
> Cc: Wayne Lin <Wayne.Lin at amd.com>
> Cc: Lyude Paul <lyude at redhat.com>
> Cc: dri-devel at lists.freedesktop.org
> Signed-off-by: Imre Deak <imre.deak at intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_mst_topology.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_dp_mst_topology.c b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> index e04f87ff755ac..5f90860d49c34 100644
> --- a/drivers/gpu/drm/display/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/display/drm_dp_mst_topology.c
> @@ -3341,7 +3341,8 @@ int drm_dp_add_payload_part1(struct drm_dp_mst_topology_mgr *mgr,
>  		(!allocate || ret < 0) ? DRM_DP_MST_PAYLOAD_ALLOCATION_LOCAL :
>  								DRM_DP_MST_PAYLOAD_ALLOCATION_DFP;
>  
> -	drm_dp_mst_topology_put_port(port);
> +	if (port)
> +		drm_dp_mst_topology_put_port(port);
>  
>  	return ret;
>  }

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat