From: Jani Nikula <jani.nikula@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: Wambui Karuga <wambui.karugax@gmail.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/display: Remove useless but deadly local
Date: Thu, 26 Mar 2020 12:03:26 +0200 [thread overview]
Message-ID: <878sjnigb5.fsf@intel.com> (raw)
In-Reply-To: <20200326082838.16357-1-chris@chris-wilson.co.uk>
On Thu, 26 Mar 2020, Chris Wilson <chris@chris-wilson.co.uk> 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@intel.com>
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@chris-wilson.co.uk>
> Cc: Wambui Karuga <wambui.karugax@gmail.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
> ---
> 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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2020-03-26 10:03 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-26 8:28 [Intel-gfx] [PATCH] drm/i915/display: Remove useless but deadly local Chris Wilson
2020-03-26 8:43 ` Jani Nikula
2020-03-26 9:12 ` [Intel-gfx] ✓ Fi.CI.BAT: success for " Patchwork
2020-03-26 10:03 ` Jani Nikula [this message]
2020-03-26 10:39 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878sjnigb5.fsf@intel.com \
--to=jani.nikula@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=intel-gfx@lists.freedesktop.org \
--cc=wambui.karugax@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.