Re: [PATCH v7 01/20] platform/x86/intel/pmt: complete pcidev to device update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing teardown of exported telemetry endpoint causes use-after-free when device is removed.
--

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

platform/x86/intel/pmt: complete pcidev to device update

This commit replaces the pci_dev pointer with a standard device pointer
in struct intel_pmt_entry to complete the transition to the device
struct. It also updates the callback mechanisms to use the updated entry
rather than the telemetry endpoint.

> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index a0ece4fc3837d..258cb460e61c2 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -42,7 +41,7 @@ struct intel_pmt_header {
>  
>  struct intel_pmt_entry {
>  	struct telem_endpoint	*ep;
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: Critical]
This is a pre-existing issue and was not introduced by this patch, but does
the teardown of this telemetry endpoint in 
drivers/platform/x86/intel/pmt/telemetry.c lack a mechanism to safely fence
off clients that still hold an active reference?

When a device is removed, pmt_telem_remove() is called:

drivers/platform/x86/intel/pmt/telemetry.c:pmt_telem_remove() {
	for (i = 0; i < priv->num_entries; i++) {
		struct intel_pmt_entry *entry = &priv->entry[i];

		kref_put(&entry->ep->kref, pmt_telem_ep_release);
		intel_pmt_dev_destroy(entry, &pmt_telem_ns);
	}
}

This unregisters the PMT device, which frees the devm-managed ep->base memory
map, but fails to set ep->present = false.

Because a client may still hold a kref, the ep object is not freed.

If the client then attempts a telemetry read, it will pass the !ep->present
check in pmt_telem_read():

drivers/platform/x86/intel/pmt/telemetry.c:pmt_telem_read() {
	if (!ep->present)
		return -ENODEV;
...
	pmt_telem_read_mmio(ep->dev, ep->cb, ep->header.guid, data, ep->base,
			    offset, NUM_BYTES_QWORD(count));
}

Could this result in an unsafe memory access to the unmapped ep->base,
leading to an MMIO fault and a kernel panic?

> -	struct pci_dev		*pcidev;
> +	struct device		*dev;
>  	struct intel_pmt_header	header;
>  	u32			disc_header[PMT_DISC_DWORDS];
>  	struct bin_attribute	pmt_bin_attr;

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