[RFC][PATCH] drm: i915: do not NULL deref hdmi attached_connector
Jani Nikula
jani.nikula at linux.intel.com
Thu Nov 14 15:53:46 UTC 2024
On Wed, 13 Nov 2024, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Wed, 13 Nov 2024, Sergey Senozhatsky <senozhatsky at chromium.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.
I'm hoping [1] would solve the issue.
BR,
Jani.
[1] https://lore.kernel.org/r/cover.1731599468.git.jani.nikula@intel.com
--
Jani Nikula, Intel
More information about the dri-devel
mailing list