linux-kernel - Re: [RFC][PATCH] drm: i915: do not NULL deref hdmi attached

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87msi3bidr.fsf@intel.com>
Date: Wed, 13 Nov 2024 11:19:44 +0200
From: Jani Nikula <jani.nikula@...ux.intel.com>
To: Sergey Senozhatsky <senozhatsky@...omium.org>, Sergey Senozhatsky
 <senozhatsky@...omium.org>
Cc: David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Rodrigo Vivi <rodrigo.vivi@...el.com>, Joonas Lahtinen
 <joonas.lahtinen@...ux.intel.com>, Tvrtko Ursulin <tursulin@...ulin.net>,
 intel-gfx@...ts.freedesktop.org, intel-xe@...ts.freedesktop.org,
 dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 ville.syrjala@...ux.intel.com
Subject: Re: [RFC][PATCH] drm: i915: do not NULL deref hdmi attached_connector

On Wed, 13 Nov 2024, Sergey Senozhatsky <senozhatsky@...omium.org> wrote:
> On (24/10/31 19:51), Sergey Senozhatsky wrote:
>> intel_ddi_init() may skip connector initialization, for instance,
>> both intel_ddi_init_dp_connector() and intel_ddi_init_hdmi_connector()
>> are optional.  This leads to situation that ->attached_connector may
>> be NULL for some connectors.  For instance, on my setup 'DDI A/PHY A'
>> and 'DDI TC1/PHY TC1' are not initialized.
>> 
>> However, functions like intel_dp_dual_mode_set_tmds_output() and
>> friends don't take this into consideration.  This leads to NULL
>> ptr-derefs:
>> 
>> KASAN: null-ptr-deref in range [0x0000000000000848-0x000000000000084f]
>> RIP: 0010:intel_hdmi_encoder_shutdown+0x105/0x230
>> Call Trace:
>> <TASK>
>> i915_driver_shutdown+0x2d8/0x490
>> pci_device_shutdown+0x83/0x150
>> device_shutdown+0x4ad/0x660
>> __se_sys_reboot+0x29c/0x4d0
>> do_syscall_64+0x60/0x90
>> 
>> Add a new helper to avoid NULL ->attached_connector derefs and
>> switch some intel_hdmi function to it.  I'm not sure if we need
>> to switch all or just intel_dp_dual_mode_set_tmds_output() (I
>> have only seen this one doing NULL derefs so far).
>
> Folks, any more comments / opinions on this?
> What should be the way forward?

Ville, we handle intel_ddi_init_dp_connector() failures but not
intel_ddi_init_hdmi_connector() failures. Do you recall if there's a
reason for that? Something like a dual-mode port where DP works but HDMI
gets rejected because of bogus VBT info?

My gut feeling is to propagate errors from intel_hdmi_init_connector()
and handle them properly in g4x_hdmi_init() and
intel_ddi_init_hdmi_connector().

Of course, we have cases where hdmi is just not initialized on DDI, and
those should be handled. But I don't think hdmi->attached_connector !=
NULL is really the right check for that.


BR,
Jani.


-- 
Jani Nikula, Intel