Re: [PATCH v7 05/20] platform/x86/intel/pmt: Do not remap when using callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional sysfs `mmap` support allows userspace to bypass hardware synchronization and power management.
--

commit 7fda3c54de1f0a266f9aa7c8acdc3ab9ae36340e
Author: Michael J. Ruhl <michael.j.ruhl@intel.com>
Subject: platform/x86/intel/pmt: Do not remap when using callbacks

A callback indicates that the parent driver is responsible for accessing
the data area, making a PMT memory remap redundant. This commit skips
creating a remap if a read_telem callback has been provided.

> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 402d51df834a6..1c7a372b4fc42 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
[ ... ]
> @@ -403,14 +403,19 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
>  	if (!entry->size)
>  		return 0;
>  
> -	res.start = entry->base_addr;
> -	res.end = res.start + entry->size - 1;
> -	res.flags = IORESOURCE_MEM;
> +	/*
> +	 * The read_telem callback is responsible for this mapping, and may have
> +	 * different requirements for use. If the callback is present do not
> +	 * create the map.
> +	 */
> +	if (!(entry->cb && entry->cb->read_telem)) {
> +		struct resource res = DEFINE_RES_MEM(entry->base_addr, entry->size);
>  
> -	entry->base = devm_ioremap_resource(dev, &res);
> -	if (IS_ERR(entry->base)) {
> -		ret = PTR_ERR(entry->base);
> -		goto fail_ioremap;
> +		entry->base = devm_ioremap_resource(dev, &res);
> +		if (IS_ERR(entry->base)) {
> +			ret = PTR_ERR(entry->base);
> +			goto fail_ioremap;
> +		}
>  	}
>  
>  	sysfs_bin_attr_init(&entry->pmt_bin_attr);
>  	entry->pmt_bin_attr.attr.name = ns->name;
>  	entry->pmt_bin_attr.attr.mode = 0440;
>  	entry->pmt_bin_attr.mmap = intel_pmt_mmap;

[Severity: High]
Does unconditionally assigning intel_pmt_mmap in intel_pmt_dev_register() allow
userspace to bypass the read_telem callback requirements?

Since intel_pmt_mmap() in drivers/platform/x86/intel/pmt/class.c directly maps
the physical BAR memory into userspace:

drivers/platform/x86/intel/pmt/class.c:intel_pmt_mmap() {
    ...
    if (io_remap_pfn_range(vma, vma->vm_start, pfn, vsize, vma->vm_page_prot))
    ...
}

any process with read permissions can mmap the sysfs attribute. If the process
then reads this mapping, won't it entirely bypass the parent driver's power
management (like xe_pm_runtime_get_if_active) and hardware synchronization?

If the device is in a low-power state, an unpowered MMIO read might cause a
system-level PCIe bus hang, or read corrupt state if the SoC re-mapper is not
configured.

Should mmap support on the sysfs attribute be disabled when a read_telem
callback is present?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909154739.1515514-22-michael.j.ruhl@intel.com?part=5