Thanks for catching all of this! for the whole series:
Reviewed-by: Lyude Paul <lyude@xxxxxxxxxx>
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@xxxxxxx>
> Cc: Lyude Paul <lyude@xxxxxxxxxx>
> Cc: dri-devel@xxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> 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
