* [PATCH 1/3] platform/x86/intel/pmt: Fix NULL dereference when reading crashlog data
2026-10-01 22:03 [PATCH 0/3] platform/x86/intel/pmt: Fix crashlog regressions and add completion uevent David E. Box
@ 2026-10-01 22:03 ` David E. Box
2026-10-02 12:43 ` Ruhl, Michael J
2026-10-01 22:03 ` [PATCH 2/3] platform/x86/intel/vsec: Fix inverted walk_header() test in get_features() David E. Box
2026-10-01 22:03 ` [PATCH 3/3] platform/x86/intel/pmt: Notify userspace when crashlogs complete David E. Box
2 siblings, 1 reply; 5+ messages in thread
From: David E. Box @ 2026-10-01 22:03 UTC (permalink / raw)
To: ilpo.jarvinen, david.e.box, linux-kernel, platform-driver-x86,
rodrigo.vivi, michael.j.ruhl, ayaz.siddiqui,
syed.abdul.muqthyar.ahmed, intel-xe, hansg
Cc: stable
Commit 353042d54d82 ("platform/x86/intel/vsec: Switch exported helpers from
pci_dev to device") changed intel_pmt_read() to pass entry->ep->dev to
pmt_telem_read_mmio() in place of entry->pcidev. entry->ep is only
allocated by the telemetry namespace's pmt_add_endpoint() hook. Crashlog
entries never get one, so any read() of a crashlog sysfs data file
dereferences a NULL pointer.
Use the intel_vsec_device parent device instead. The PMT class device is a
child of the auxiliary device, so derive it the same way
intel_pmt_attr_visible() does. This is the same device telemetry stored in
ep->dev, so behavior for telemetry and any read_telem() callback is
unchanged.
Fixes: 353042d54d82 ("platform/x86/intel/vsec: Switch exported helpers from pci_dev to device")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/pmt/class.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index d0ab8e33c62a..b69c79785d9c 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -90,6 +90,8 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
struct intel_pmt_entry *entry = container_of(attr,
struct intel_pmt_entry,
pmt_bin_attr);
+ struct device *dev = kobj_to_dev(kobj);
+ struct intel_vsec_device *ivdev = auxdev_to_ivdev(to_auxiliary_dev(dev->parent));
if (off < 0)
return -EINVAL;
@@ -100,7 +102,7 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
if (count > entry->size - off)
count = entry->size - off;
- count = pmt_telem_read_mmio(entry->ep->dev, entry->cb, entry->header.guid, buf,
+ count = pmt_telem_read_mmio(ivdev->dev, entry->cb, entry->header.guid, buf,
entry->base, off, count);
return count;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH 1/3] platform/x86/intel/pmt: Fix NULL dereference when reading crashlog data
2026-10-01 22:03 ` [PATCH 1/3] platform/x86/intel/pmt: Fix NULL dereference when reading crashlog data David E. Box
@ 2026-10-02 12:43 ` Ruhl, Michael J
0 siblings, 0 replies; 5+ messages in thread
From: Ruhl, Michael J @ 2026-10-02 12:43 UTC (permalink / raw)
To: David E. Box, ilpo.jarvinen, linux-kernel, platform-driver-x86,
Vivi, Rodrigo, Siddiqui, Ayaz A, Muqthyar Ahmed, Syed Abdul,
intel-xe, hansg
Cc: stable
>-----Original Message-----
>From: David E. Box <david.e.box@linux.intel.com>
>Sent: Thursday, October 1, 2026 6:04 PM
>To: ilpo.jarvinen@linux.intel.com; david.e.box@linux.intel.com; linux-
>kernel@vger.kernel.org; platform-driver-x86@vger.kernel.org; Vivi, Rodrigo
><rodrigo.vivi@intel.com>; Ruhl, Michael J <michael.j.ruhl@intel.com>; Siddiqui,
>Ayaz A <ayaz.siddiqui@intel.com>; Muqthyar Ahmed, Syed Abdul
><syed.abdul.muqthyar.ahmed@intel.com>; intel-xe@lists.freedesktop.org;
>hansg@kernel.org
>Cc: stable@vger.kernel.org
>Subject: [PATCH 1/3] platform/x86/intel/pmt: Fix NULL dereference when
>reading crashlog data
>
>Commit 353042d54d82 ("platform/x86/intel/vsec: Switch exported helpers
>from
>pci_dev to device") changed intel_pmt_read() to pass entry->ep->dev to
>pmt_telem_read_mmio() in place of entry->pcidev. entry->ep is only
>allocated by the telemetry namespace's pmt_add_endpoint() hook. Crashlog
>entries never get one, so any read() of a crashlog sysfs data file
>dereferences a NULL pointer.
>
>Use the intel_vsec_device parent device instead. The PMT class device is a
>child of the auxiliary device, so derive it the same way
>intel_pmt_attr_visible() does. This is the same device telemetry stored in
>ep->dev, so behavior for telemetry and any read_telem() callback is
>unchanged.
>
>Fixes: 353042d54d82 ("platform/x86/intel/vsec: Switch exported helpers from
>pci_dev to device")
>Cc: stable@vger.kernel.org
>Assisted-by: LLM
>Signed-off-by: David E. Box <david.e.box@linux.intel.com>
>---
> drivers/platform/x86/intel/pmt/class.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/platform/x86/intel/pmt/class.c
>b/drivers/platform/x86/intel/pmt/class.c
>index d0ab8e33c62a..b69c79785d9c 100644
>--- a/drivers/platform/x86/intel/pmt/class.c
>+++ b/drivers/platform/x86/intel/pmt/class.c
>@@ -90,6 +90,8 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
> struct intel_pmt_entry *entry = container_of(attr,
> struct intel_pmt_entry,
> pmt_bin_attr);
>+ struct device *dev = kobj_to_dev(kobj);
>+ struct intel_vsec_device *ivdev =
>auxdev_to_ivdev(to_auxiliary_dev(dev->parent));
>
> if (off < 0)
> return -EINVAL;
>@@ -100,7 +102,7 @@ intel_pmt_read(struct file *filp, struct kobject *kobj,
> if (count > entry->size - off)
> count = entry->size - off;
>
>- count = pmt_telem_read_mmio(entry->ep->dev, entry->cb, entry-
>>header.guid, buf,
>+ count = pmt_telem_read_mmio(ivdev->dev, entry->cb, entry-
>>header.guid, buf,
> entry->base, off, count);
Hi Ilpo, David,
My patch:
[PATCH v11 01/20] platform/x86/intel/pmt: complete pcidev to device update
Fixes this issue in a slightly different way... (uses entry->dev rather than entry->pcidev).
David,
In my patch I have also updated the intel_pmt_get_features() function to use the
entry->dev value (rather than entry->ep->dev).
I think the ep->dev is for Telemetry access, and "features" is a general usage?
Should the _get_features stay entry->ep->dev? or is entry->dev "more correct"?
Thanks
Mike
> return count;
>--
>2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/3] platform/x86/intel/vsec: Fix inverted walk_header() test in get_features()
2026-10-01 22:03 [PATCH 0/3] platform/x86/intel/pmt: Fix crashlog regressions and add completion uevent David E. Box
2026-10-01 22:03 ` [PATCH 1/3] platform/x86/intel/pmt: Fix NULL dereference when reading crashlog data David E. Box
@ 2026-10-01 22:03 ` David E. Box
2026-10-01 22:03 ` [PATCH 3/3] platform/x86/intel/pmt: Notify userspace when crashlogs complete David E. Box
2 siblings, 0 replies; 5+ messages in thread
From: David E. Box @ 2026-10-01 22:03 UTC (permalink / raw)
To: ilpo.jarvinen, david.e.box, linux-kernel, platform-driver-x86,
rodrigo.vivi, michael.j.ruhl, ayaz.siddiqui,
syed.abdul.muqthyar.ahmed, intel-xe, hansg
Cc: stable
intel_vsec_walk_header() used to return a bool that was true when devices
were found. It was converted to return 0 on success and a negative errno
on failure, but the boolean test in intel_vsec_get_features() was left
unchanged, so its meaning is now inverted.
A successful walk returns 0 and leaves found set to false, while a failed
walk returns an error that evaluates to true. When no other capabilities
are present, intel_vsec_pci_init() then returns -ENODEV even though the
auxiliary devices were created, and the driver core tears them back down.
This affects platforms that have no DVSEC or VSEC capabilities and
describe their features through device_data instead, which today means
DG1. Test the return value for success explicitly.
Fixes: a6ce8bf3c993 ("platform/x86/intel/vsec: Return real error codes from registration path")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
drivers/platform/x86/intel/vsec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel/vsec.c b/drivers/platform/x86/intel/vsec.c
index 5ab2215fdd7f..7d51f27f09bc 100644
--- a/drivers/platform/x86/intel/vsec.c
+++ b/drivers/platform/x86/intel/vsec.c
@@ -641,7 +641,7 @@ static bool intel_vsec_get_features(struct pci_dev *pdev,
found = true;
if (info && (info->quirks & VSEC_QUIRK_NO_DVSEC) &&
- intel_vsec_walk_header(&pdev->dev, info))
+ !intel_vsec_walk_header(&pdev->dev, info))
found = true;
return found;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 3/3] platform/x86/intel/pmt: Notify userspace when crashlogs complete
2026-10-01 22:03 [PATCH 0/3] platform/x86/intel/pmt: Fix crashlog regressions and add completion uevent David E. Box
2026-10-01 22:03 ` [PATCH 1/3] platform/x86/intel/pmt: Fix NULL dereference when reading crashlog data David E. Box
2026-10-01 22:03 ` [PATCH 2/3] platform/x86/intel/vsec: Fix inverted walk_header() test in get_features() David E. Box
@ 2026-10-01 22:03 ` David E. Box
2 siblings, 0 replies; 5+ messages in thread
From: David E. Box @ 2026-10-01 22:03 UTC (permalink / raw)
To: ilpo.jarvinen, david.e.box, linux-kernel, platform-driver-x86,
rodrigo.vivi, michael.j.ruhl, ayaz.siddiqui,
syed.abdul.muqthyar.ahmed, intel-xe, hansg
A crashlog may already be complete when the driver binds, or it may
complete asynchronously after userspace requests a manual trigger.
Userspace otherwise has to poll to discover that data is ready.
Emit a KOBJ_CHANGE uevent on the per-instance crashlog device when a
completed log is found at probe. After a manual trigger, poll the
completion bit at 100 ms intervals for up to 5 seconds and emit the same
event when capture completes. The event carries two environment
variables:
INTEL_PMT_CRASHLOG_EVENT=PRESENT
INTEL_PMT_CRASHLOG_COMPLETE=1
Initialize the work items before the device's sysfs attributes are
exposed, and disable and drain them during removal.
Document the new uevent in the sysfs-class-intel_pmt ABI file.
Assisted-by: LLM
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
.../ABI/testing/sysfs-class-intel_pmt | 14 ++++
drivers/platform/x86/intel/pmt/crashlog.c | 84 ++++++++++++++++++-
2 files changed, 95 insertions(+), 3 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-class-intel_pmt b/Documentation/ABI/testing/sysfs-class-intel_pmt
index ed4c886a21b1..c0b0d123afdf 100644
--- a/Documentation/ABI/testing/sysfs-class-intel_pmt
+++ b/Documentation/ABI/testing/sysfs-class-intel_pmt
@@ -66,6 +66,20 @@ Description:
can be determined from an XML file of specified GUID for the
parent device.
+What: /sys/class/intel_pmt/crashlog<x>
+Date: October 2026
+KernelVersion: 7.4
+Contact: David Box <david.e.box@linux.intel.com>
+Description:
+ When a crashlog device is probed with a completed crashlog
+ already present, or a manual trigger completes, the driver emits
+ a KOBJ_CHANGE uevent for that crashlog<x> device.
+
+ The following uevent environment variables are added:
+
+ INTEL_PMT_CRASHLOG_EVENT=PRESENT
+ INTEL_PMT_CRASHLOG_COMPLETE=1
+
What: /sys/class/intel_pmt/crashlog<x>/crashlog
Date: October 2020
KernelVersion: 5.10
diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index f936daf99e4d..21e8e2199bdc 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -12,12 +12,14 @@
#include <linux/cleanup.h>
#include <linux/intel_vsec.h>
#include <linux/kernel.h>
+#include <linux/kobject.h>
#include <linux/module.h>
#include <linux/mutex.h>
#include <linux/pci.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
#include <linux/overflow.h>
+#include <linux/workqueue.h>
#include "class.h"
@@ -113,6 +115,9 @@ struct crashlog_entry {
struct intel_pmt_entry entry;
struct mutex control_mutex;
const struct crashlog_info *info;
+ struct work_struct uevent_work;
+ struct delayed_work uevent_poll_work;
+ u8 uevent_poll_tries;
};
struct pmt_crashlog_priv {
@@ -120,6 +125,9 @@ struct pmt_crashlog_priv {
struct crashlog_entry entry[];
};
+#define PMT_CRASHLOG_UEVENT_POLL_MS 100
+#define PMT_CRASHLOG_UEVENT_POLL_MAX_TRIES 50
+
/*
* I/O
*/
@@ -227,6 +235,47 @@ static void pmt_crashlog_set_rearm(struct crashlog_entry *crashlog)
pmt_crashlog_rmw(crashlog, crashlog->info->control.rearm, true);
}
+static void pmt_crashlog_uevent_fn(struct work_struct *work)
+{
+ struct crashlog_entry *crashlog =
+ container_of(work, struct crashlog_entry, uevent_work);
+ char *envp[] = {
+ "INTEL_PMT_CRASHLOG_EVENT=PRESENT",
+ "INTEL_PMT_CRASHLOG_COMPLETE=1",
+ NULL,
+ };
+
+ if (crashlog->entry.kobj)
+ kobject_uevent_env(crashlog->entry.kobj, KOBJ_CHANGE, envp);
+}
+
+static void pmt_crashlog_uevent_poll_fn(struct work_struct *work)
+{
+ struct crashlog_entry *crashlog =
+ container_of(to_delayed_work(work), struct crashlog_entry,
+ uevent_poll_work);
+ guard(mutex)(&crashlog->control_mutex);
+
+ /* A newer trigger_store() re-armed us; that cycle owns the notification */
+ if (delayed_work_pending(&crashlog->uevent_poll_work))
+ return;
+
+ if (pmt_crashlog_complete(crashlog)) {
+ schedule_work(&crashlog->uevent_work);
+ return;
+ }
+
+ if (++crashlog->uevent_poll_tries < PMT_CRASHLOG_UEVENT_POLL_MAX_TRIES)
+ schedule_delayed_work(&crashlog->uevent_poll_work,
+ msecs_to_jiffies(PMT_CRASHLOG_UEVENT_POLL_MS));
+}
+
+static void pmt_crashlog_notify_pending(struct crashlog_entry *crashlog)
+{
+ if (pmt_crashlog_complete(crashlog))
+ schedule_work(&crashlog->uevent_work);
+}
+
/*
* sysfs
*/
@@ -423,7 +472,19 @@ trigger_store(struct device *dev, struct device_attribute *attr,
if (pmt_crashlog_complete(crashlog))
return -EEXIST;
+ /*
+ * Only now are we actually starting a fresh crash: any stale poll
+ * cycle left over from a prior trigger can be dropped safely, since
+ * pmt_crashlog_complete() above proved it carried no unresolved
+ * completion. Non-sync: uevent_poll_fn() takes this same mutex, so
+ * cancel_delayed_work_sync() here could deadlock against it.
+ */
+ cancel_delayed_work(&crashlog->uevent_poll_work);
+
pmt_crashlog_set_execute(crashlog);
+ crashlog->uevent_poll_tries = 0;
+ schedule_delayed_work(&crashlog->uevent_poll_work,
+ msecs_to_jiffies(PMT_CRASHLOG_UEVENT_POLL_MS));
return count;
}
@@ -551,6 +612,9 @@ static void pmt_crashlog_remove(struct auxiliary_device *auxdev)
for (i = 0; i < priv->num_entries; i++) {
struct crashlog_entry *crashlog = &priv->entry[i];
+ /* Disable, not cancel: a racing trigger_store() must not re-arm */
+ disable_delayed_work_sync(&crashlog->uevent_poll_work);
+ disable_work_sync(&crashlog->uevent_work);
intel_pmt_dev_destroy(&crashlog->entry, &pmt_crashlog_ns);
mutex_destroy(&crashlog->control_mutex);
}
@@ -572,15 +636,29 @@ static int pmt_crashlog_probe(struct auxiliary_device *auxdev,
auxiliary_set_drvdata(auxdev, priv);
for (i = 0; i < intel_vsec_dev->num_resources; i++) {
- struct intel_pmt_entry *entry = &priv->entry[priv->num_entries].entry;
+ struct crashlog_entry *crashlog = &priv->entry[priv->num_entries];
+ struct intel_pmt_entry *entry = &crashlog->entry;
+
+ /* init before dev_create() exposes trigger sysfs to userspace */
+ INIT_WORK(&crashlog->uevent_work, pmt_crashlog_uevent_fn);
+ INIT_DELAYED_WORK(&crashlog->uevent_poll_work,
+ pmt_crashlog_uevent_poll_fn);
+ crashlog->uevent_poll_tries = 0;
ret = intel_pmt_dev_create(entry, &pmt_crashlog_ns, intel_vsec_dev, i);
- if (ret < 0)
+ if (ret < 0) {
+ cancel_delayed_work_sync(&crashlog->uevent_poll_work);
+ cancel_work_sync(&crashlog->uevent_work);
goto abort_probe;
- if (ret)
+ }
+ if (ret) {
+ cancel_delayed_work_sync(&crashlog->uevent_poll_work);
+ cancel_work_sync(&crashlog->uevent_work);
continue;
+ }
priv->num_entries++;
+ pmt_crashlog_notify_pending(crashlog);
}
return 0;
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread