RE: [PATCH] drm/xe/hdcp: Add NULL check for media_gt in intel_hdcp_gsc_check_status()

[Gustavo Sousa <gustavo.sousa@intel.com>]

"Lin, Shuicheng" <shuicheng.lin@intel.com> writes:

> On Tue, Mar 3, 2026 5:56 AM Gustavo Sousa wrote:
>> "Lin, Shuicheng" <shuicheng.lin@intel.com> writes:
>> 
>> > On Mon, Mar 2, 2026 10:19 AM Gustavo Sousa wrote:
>> >> When media GT is disabled via configfs, there is no allocation for
>> >> media_gt, which is kept as NULL.  In such scenario,
>> >> intel_hdcp_gsc_check_status() results in a kernel pagefault error due
>> >> to &gt-
>> >> >uc.gsc being evaluated as an invalid memory address.
>> >>
>> >> Fix that by introducing a NULL check on media_gt and bailing out early if so.
>> >>
>> >> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> >> ---
>> >>  drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 6 ++++++
>> >>  1 file changed, 6 insertions(+)
>> >>
>> >> diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> >> b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> >> index 29c72aa4b0d2..1cac00c17d4c 100644
>> >> --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> >> +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
>> >> @@ -39,6 +39,12 @@ static bool intel_hdcp_gsc_check_status(struct
>> >> drm_device *drm)
>> >>  	struct xe_gt *gt = tile->media_gt;
>> >>  	struct xe_gsc *gsc = &gt->uc.gsc;
>> >
>> > If gt is NULL, this "&gt->uc.gsc" is already undefined behavior.
>> 
>> I don't think so. It just turns out that the offset will be invalid. We are not
>> dereferencing the gt pointer here.
>
> I also thought it should be the offset of the structure, like offsetof() do.
> But it turns out not correct when I ask it to chatgpt. And here is an article to explain it:
> https://medium.com/@Code_Analysis/null-pointer-dereferencing-causes-undefined-behavior-32462a200106
>
> So, I still prefer to move the assignment to after the !gt check. 😊

I don't have access to the C standard, but the article and its
linked material sound convincing.  Thanks for sharing that!

I'll incorporate your suggestion.

--
Gustavo Sousa

>
> Shuicheng
>
>> 
>> > I would prefer to move this assignment to after below check.
>> 
>> I think that might be unecessary. Would you agree with keeping it as-is based
>> on my argument above?
>> 
>> >
>> >>
>> >> +	if (!gt) {
>> >> +		drm_dbg_kms(&xe->drm,
>> >> +			    "not checking GSC status for HDCP2.x: media GT not
>> >> present or disabled\n");
>> >> +		return false;
>> >> +	}
>> >> +
>> >>  	if (!gsc || !xe_uc_fw_is_available(&gsc->fw)) {
>> >
>> > BTW, if gt is not NULL, then gsc cannot be NULL. So this "!gsc" check
>> > could be dropped.
>> 
>> Yep. Makes sense.
>> 
>> --
>> Gustavo Sousa
>> 
>> >
>> > Shuicheng
>> >
>> >>  		drm_dbg_kms(&xe->drm,
>> >>  			    "GSC Components not ready for HDCP2.x\n");
>> >>
>> >> ---
>> >> base-commit: 40e52ed6c66cef091579dd7fe49ccd2de135f5c5
>> >> change-id: 20260302-check-for-null-media_gt-in-
>> >> intel_hdcp_gsc_check_status-071898b8a402
>> >>
>> >> Best regards,
>> >> --
>> >> Gustavo Sousa <gustavo.sousa@intel.com>