On Thu, 26 Mar 2020, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote:
> Beware dereferencing the NULL pointer prior to checking for its
> existence.
Okay, so the crt code calls ddi functions, and enc_to_dig_port() will
return NULL. Backtrace for posterity below, copy-pasted from logs Chris
pointed me at.
I think alternatively we could just use to_i915(encoder->base.dev) here
instead of accessing intel_dig_port, but the patch at hand fixes stuff,
Reviewed-by: Jani Nikula <jani.nikula@xxxxxxxxx>
Some other observations: As we're moving further and further away from
crt, I don't think people expect the intel_ddi_* functions to be called
directly from crt code, or any code where the encoder is *not* a digital
port.
And that brings us to another case where enc_to_dig_port() will return
NULL: INTEL_OUTPUT_DP_MST. We rarely check for enc_to_dig_port() being
NULL anywhere. There's probably more subtle bugs waiting to happen here
and there.
BR,
Jani.
<1>[ 3.324694] BUG: kernel NULL pointer dereference, address: 0000000000000000
<1>[ 3.324696] #PF: supervisor read access in kernel mode
<4>[ 3.324704] hardirqs last enabled at (751): [<ffffffff812a4f77>] d_lookup+0x57/0xa0
<1>[ 3.324709] #PF: error_code(0x0000) - not-present page
<4>[ 3.324716] hardirqs last disabled at (752): [<ffffffff8125a1b9>] __slab_alloc.isra.89.constprop.94+0x19/0x70
<4>[ 3.324720] softirqs last enabled at (402): [<ffffffff81e00385>] __do_softirq+0x385/0x47f
<6>[ 3.324725] PGD 0 P4D 0
<4>[ 3.324733] softirqs last disabled at (395): [<ffffffff810bab6a>] irq_exit+0xba/0xc0
<4>[ 3.324762] Oops: 0000 [#1] PREEMPT SMP PTI
<4>[ 3.324768] CPU: 0 PID: 380 Comm: systemd-udevd Not tainted 5.6.0-rc7-CI-CI_DRM_8189+ #1
<4>[ 3.324776] Hardware name: MSI MS-7924/Z97M-G43(MS-7924), BIOS V1.12 02/15/2016
<4>[ 3.324840] RIP: 0010:intel_read_infoframe+0x3a/0x170 [i915]
<4>[ 3.324848] Code: 65 48 8b 04 25 28 00 00 00 48 89 44 24 20 31 c0 83 f9 0a 77 12 ba 01 00 00 00 48 d3 e2 f7 c2 c0 05 00 00 48 0f 45 c7 83 fb 03 <4c> 8b 20 0f 84 f2 00 00 00 83 fb 0a 0f 84 f3 00 00 00 83 fb 07 0f
<4>[ 3.324865] RSP: 0018:ffffc900005438b0 EFLAGS: 00010212
<4>[ 3.324871] RAX: 0000000000000000 RBX: 0000000000000082 RCX: 0000000000000001
<4>[ 3.324879] RDX: 0000000000000002 RSI: ffff8883f8309000 RDI: ffff8883f80cbe00
<4>[ 3.324887] RBP: ffff8883f8309b34 R08: 000000000000000e R09: 0000000000000001
<4>[ 3.324894] R10: 0000000000000000 R11: 0000000000000000 R12: ffff8883f7ac0000
<4>[ 3.324901] R13: ffff8883f7ac0000 R14: ffff8883f7ac0d90 R15: ffff8883f844d000
<4>[ 3.324908] FS: 00007ffa4a839680(0000) GS:ffff888410000000(0000) knlGS:0000000000000000
<4>[ 3.324917] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[ 3.324923] CR2: 0000000000000000 CR3: 0000000401aa4002 CR4: 00000000001606f0
<4>[ 3.324930] Call Trace:
<4>[ 3.324980] ? gen6_read32+0x272/0x300 [i915]
<4>[ 3.325044] intel_ddi_get_config+0x238/0x610 [i915]
<4>[ 3.325108] hsw_crt_get_config+0x12/0x40 [i915]
<4>[ 3.325173] intel_modeset_setup_hw_state+0x3b3/0x1660 [i915]
<4>[ 3.325182] ? ww_mutex_lock+0x39/0x70
<4>[ 3.325190] ? drm_modeset_lock+0xad/0x120
<4>[ 3.325254] intel_modeset_init+0x582/0x1c50 [i915]
<4>[ 3.325263] ? _raw_spin_unlock_irqrestore+0x34/0x60
<4>[ 3.325314] ? intel_irq_postinstall+0xb3/0x610 [i915]
<4>[ 3.325366] i915_driver_probe+0xa7e/0xed0 [i915]
<4>[ 3.325375] ? __pm_runtime_resume+0x4f/0x80
<4>[ 3.325426] i915_pci_probe+0x43/0x1b0 [i915]
<4>[ 3.325434] ? _raw_spin_unlock_irqrestore+0x34/0x60
<4>[ 3.325442] pci_device_probe+0x9e/0x120
<4>[ 3.325450] really_probe+0xea/0x430
<4>[ 3.325456] driver_probe_device+0x10b/0x120
<4>[ 3.325463] device_driver_attach+0x4a/0x50
<4>[ 3.325469] __driver_attach+0x97/0x130
<4>[ 3.325476] ? device_driver_attach+0x50/0x50
<4>[ 3.325482] bus_for_each_dev+0x74/0xc0
<4>[ 3.325489] bus_add_driver+0x142/0x220
<4>[ 3.325495] driver_register+0x56/0xf0
<4>[ 3.325543] i915_init+0x6c/0x7c [i915]
<4>[ 3.325549] ? 0xffffffffa087f000
<4>[ 3.325555] do_one_initcall+0x58/0x300
<4>[ 3.325562] ? rcu_read_lock_sched_held+0x4d/0x80
<4>[ 3.325570] ? kmem_cache_alloc_trace+0x2a6/0x2d0
<4>[ 3.325578] do_init_module+0x56/0x1f2
<4>[ 3.325584] load_module+0x233d/0x2a30
<4>[ 3.325596] ? __do_sys_finit_module+0xe9/0x110
<4>[ 3.325602] __do_sys_finit_module+0xe9/0x110
<4>[ 3.325612] do_syscall_64+0x4f/0x220
<4>[ 3.325618] entry_SYSCALL_64_after_hwframe+0x49/0xbe
<4>[ 3.325624] RIP: 0033:0x7ffa4a35a839
<4>[ 3.325630] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89 01 48
<4>[ 3.325647] RSP: 002b:00007ffdbdf7b848 EFLAGS: 00000246 ORIG_RAX: 0000000000000139
<4>[ 3.325656] RAX: ffffffffffffffda RBX: 000055bc2aa95fc0 RCX: 00007ffa4a35a839
<4>[ 3.325663] RDX: 0000000000000000 RSI: 00007ffa4a039145 RDI: 000000000000000f
<4>[ 3.325671] RBP: 00007ffa4a039145 R08: 0000000000000000 R09: 00007ffdbdf7b960
<4>[ 3.325678] R10: 000000000000000f R11: 0000000000000246 R12: 0000000000000000
<4>[ 3.325686] R13: 000055bc2aaa3200 R14: 0000000000020000 R15: 000055bc2aa95fc0
<4>[ 3.325696] Modules linked in: i915(+) mei_hdcp x86_pkg_temp_thermal coretemp snd_hda_codec_realtek snd_hda_codec_generic crct10dif_pclmul crc32_pclmul snd_hda_intel snd_intel_dspcfg snd_hda_codec ghash_clmulni_intel snd_hwdep r8169 snd_hda_core realtek snd_pcm mei_me mei prime_numbers lpc_ich
<4>[ 3.325729] CR2: 0000000000000000
<4>[ 3.325740] ---[ end trace 22628c46d7d6a985 ]---
>
> Fixes: 419190429cd1 ("drm/i915/hdmi: use struct drm_device based logging")
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Wambui Karuga <wambui.karugax@xxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxx>
> Cc: "Ville Syrjälä" <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_hdmi.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 395dc192baa0..0076abc63851 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -691,7 +691,6 @@ void intel_read_infoframe(struct intel_encoder *encoder,
> union hdmi_infoframe *frame)
> {
> struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
> - struct drm_i915_private *i915 = to_i915(intel_dig_port->base.base.dev);
> u8 buffer[VIDEO_DIP_DATA_SIZE];
> int ret;
>
> @@ -708,13 +707,13 @@ void intel_read_infoframe(struct intel_encoder *encoder,
> /* see comment above for the reason for this offset */
> ret = hdmi_infoframe_unpack(frame, buffer + 1, sizeof(buffer) - 1);
> if (ret) {
> - drm_dbg_kms(&i915->drm,
> + drm_dbg_kms(encoder->base.dev,
> "Failed to unpack infoframe type 0x%02x\n", type);
> return;
> }
>
> if (frame->any.type != type)
> - drm_dbg_kms(&i915->drm,
> + drm_dbg_kms(encoder->base.dev,
> "Found the wrong infoframe type 0x%x (expected 0x%02x)\n",
> frame->any.type, type);
> }
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
