All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>
Cc: intel-xe@lists.freedesktop.org
Subject: Re: [PATCH v6 01/18] platform/x86/intel/pmt: complete pcidev to device update
Date: Fri, 04 Sep 2026 18:45:58 +0000	[thread overview]
Message-ID: <20260904184558.8809A1F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260904182451.1164868-21-michael.j.ruhl@intel.com>

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the telemetry endpoint struct (`ep`) can be accessed after its MMIO memory is unmapped because `ep->present` is not set to `false` during device removal.
- [High] This problem wasn't introduced by this patch, but there is an unconditional bad cast of a generic device to a PCI device using `to_pci_dev()`, causing undefined behavior for ACPI-based PMT devices.
--

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

    platform/x86/intel/pmt: complete pcidev to device update
    
    The update that moved struct pci_dev usage to struct device is
    incomplete. Only telemetry endpoints are covered.
    
    Other PMT features (crashlog) are now blocked from using the callback
    mechanism.
    
    Change struct intel_pmt_entry pci_dev member to device.
    
    Update callback usage to use the intel_pmt_entry rather than the
    telemetry endpoint.
    
    Fixes: 353042d54d82 ("platform/x86/intel/vsec: Switch exported helpers from pci_dev to device")

> 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;
> -	struct pci_dev		*pcidev;
> +	struct device		*dev;

[Severity: High]
This problem wasn't introduced by this patch, but while reviewing the changes
to struct intel_pmt_entry, I noticed a potential type confusion issue in
drivers/platform/x86/intel/pmt/telemetry.c:

drivers/platform/x86/intel/pmt/telemetry.c:pmt_copy_region() {
    struct pci_dev *pdev = to_pci_dev(entry->ep->dev);
    ...
}

Can this unconditional cast of a generic device to a PCI device cause
undefined behavior for ACPI-based PMT devices?

PMT telemetry devices can be discovered via ACPI (e.g., INTEL_VSEC_DISC_ACPI
used in pwrm_telemetry.c and ssram_telemetry.c). In this case, entry->ep->dev
points to an ACPI platform device, not a PCI device.

When pmt_copy_region() blindly calls to_pci_dev(entry->ep->dev), it uses
container_of to subtract a large offset. The resulting garbage pointer is
passed to intel_vsec_get_mapping(), which passes it to pci_match_id(). This
causes a dereference of PCI-specific struct fields like vendor and device
from unmapped or unrelated memory, leading to an out-of-bounds read and
likely kernel panic.

This appears to be reachable unconditionally when consumers like resctrl
iterate over telemetry endpoints and call intel_pmt_get_regions_by_feature()
which in turn calls pmt_copy_region() on systems using ACPI-based PMT
devices.


[Severity: High]
This problem wasn't introduced by this patch, but the telemetry endpoint
struct (ep) can be accessed after its MMIO memory is unmapped because
ep->present is not set to false during device removal.

Looking at 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);
    }

Can this lead to a use-after-free of MMIO memory?

A consumer (like resctrl or intel_pmc_core) requests a telemetry endpoint
via pmt_telem_find_and_register_endpoint(), holding a kref to the
endpoint.

If the PMT device is subsequently unbound or hot-removed,
pmt_telem_remove() drops the driver's reference. The devres framework then
unmaps entry->base (which aliases to ep->base).

Because ep->present is never set to false, the consumer will bypass the
if (!ep->present) safety checks and attempt to read from the now-unmapped
ep->base MMIO address.

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

  reply	other threads:[~2026-09-04 18:46 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-04 18:24 [PATCH v6 00/18] Crescent Island PMT support Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 01/18] platform/x86/intel/pmt: complete pcidev to device update Michael J. Ruhl
2026-09-04 18:45   ` sashiko-bot [this message]
2026-09-04 18:24 ` [PATCH v6 02/18] platform/x86/intel/pmt: refactor rmw with a return value Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 03/18] platform/x86/intel/pmt: refactor rc " Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 04/18] platform/x86/intel/pmt: Add register access callbacks Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 05/18] platform/x86/intel/pmt: Do not remap when using callbacks Michael J. Ruhl
2026-09-04 18:42   ` sashiko-bot
2026-09-04 18:24 ` [PATCH v6 06/18] drm/xe/vsec: Do not register BMG PMT for VF Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 07/18] drm/xe/vsec: Correct locking order Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 08/18] drm/xe/vsec: Use correct pm state get Michael J. Ruhl
2026-09-04 18:24 ` [PATCH v6 09/18] drm/xe/vsec: Add DOC text for VSEC Michael J. Ruhl
2026-09-04 18:25 ` [PATCH v6 10/18] drm/xe/vsec: Support possible hotplug exit Michael J. Ruhl
2026-09-04 18:25 ` [PATCH v6 11/18] drm/xe/vsec: Support Crescent Island PMT Michael J. Ruhl
2026-09-04 18:25 ` [PATCH v6 12/18] drm/xe/vsec: Refactor BattleMage PMT defines Michael J. Ruhl
2026-09-04 18:53   ` sashiko-bot
2026-09-04 19:37     ` Ruhl, Michael J
2026-09-04 18:25 ` [PATCH v6 13/18] drm/xe/vsec: Crescent Island PMT decode Michael J. Ruhl
2026-09-04 18:50   ` sashiko-bot
2026-09-04 18:25 ` [PATCH v6 14/18] drm/xe/vsec: Crescent Island PMT callbacks Michael J. Ruhl
2026-09-04 18:51   ` sashiko-bot
2026-09-04 18:25 ` [PATCH v6 15/18] drm/xe/vsec: Support late bind fw information Michael J. Ruhl
2026-09-04 18:56   ` sashiko-bot
2026-09-04 18:25 ` [PATCH v6 16/18] drm/xe/vsec: Add PMT GUID internal access Michael J. Ruhl
2026-09-04 18:25 ` [PATCH v6 17/18] drm/xe/vsec: Update PMT " Michael J. Ruhl
2026-09-04 18:55   ` sashiko-bot
2026-09-04 18:25 ` [PATCH v6 18/18] drm/xe/vsec: Refactor platform check Michael J. Ruhl
2026-09-04 18:55 ` ✗ CI.checkpatch: warning for Crescent Island PMT support (rev8) Patchwork
2026-09-04 18:56 ` ✗ CI.KUnit: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260904184558.8809A1F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=intel-xe@lists.freedesktop.org \
    --cc=michael.j.ruhl@intel.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.