Re: [PATCH v13 01/16] drm/xe/vsec: Do not register BMG PMT for VF

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/xe/vsec: NULL pointer dereference in xe_pmt_telem_read() in survivability mode
--

commit ce97359d953d35ffff16565daa34557b5236e227
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>

drm/xe/vsec: Do not register BMG PMT for VF

This commit skips VSEC PMT registration on Battlemage SR-IOV VF devices,
adds a VF assertion to xe_pmt_telem_read(), and removes the NULL check on
xe->soc_remapper.set_telem_region.

> diff --git a/drivers/gpu/drm/xe/xe_vsec.c b/drivers/gpu/drm/xe/xe_vsec.c
> index a9baf0bfe572c..178f22db40461 100644
> --- a/drivers/gpu/drm/xe/xe_vsec.c
> +++ b/drivers/gpu/drm/xe/xe_vsec.c
> @@ -149,6 +149,8 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  	u32 offset;
>  	int ret;
>  
> +	xe_assert(xe, !IS_SRIOV_VF(xe));
> +
>  	ret = xe_guid_decode(guid, &mem_region, &offset);
>  	if (ret)
>  		return ret;
> @@ -157,9 +159,6 @@ int xe_pmt_telem_read(struct device *dev, u32 guid, u64 *data, loff_t user_offse
>  
>  	guard(mutex)(&xe->pmt.lock);
>  
> -	if (!xe->soc_remapper.set_telem_region)
> -		return -ENODEV;
> -

[Severity: High]
Can removing the xe->soc_remapper.set_telem_region check here cause a NULL
function pointer dereference in xe_pmt_telem_read() when a Battlemage PF
enters boot survivability mode?

When boot survivability mode is enabled during xe_device_probe_early(),
enable_boot_survivability_mode() initializes VSEC and registers the PMT
callbacks:

drivers/gpu/drm/xe/xe_survivability_mode.c:enable_boot_survivability_mode() {
	...
	xe_heci_gsc_init(xe);

	xe_vsec_init(xe);
	...
}

Then __xe_pci_probe() returns early before calling xe_device_probe():

drivers/gpu/drm/xe/xe_pci.c:__xe_pci_probe() {
	...
	err = xe_device_probe_early(xe);
	if (xe_survivability_mode_is_boot_enabled(xe))
		return 0;
	...
	err = xe_device_probe(xe);
}

Because xe_soc_remapper_init(xe) is only called inside xe_device_probe(),
xe->soc_remapper.set_telem_region remains NULL in boot survivability mode.
Meanwhile, local_pci_probe() holds a runtime PM reference, so
xe_pm_runtime_get_if_active(xe) returns true. If userspace reads the
exposed PMT telemetry or crashlog sysfs attributes, xe_pmt_telem_read()
will call xe->soc_remapper.set_telem_region(xe, mem_region) when it is
NULL.

It looks like a later patch in the series, commit 6e126caaac7b
("drm/xe/vsec: Support survivability mode for CRI"), adds a call to
xe_soc_remapper_init(xe) in enable_boot_survivability_mode(), but at this
point in the series (and if that change is not backported together with
this one), reading PMT sysfs files in boot survivability mode will crash.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102221314.119028-18-michael.j.ruhl@intel.com?part=1