RE: [PATCH v4 01/10] platform/x86/intel/pmt: fix a crashlog NULL pointer access

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3860 bytes --]

On Wed, 11 Jun 2025, Ruhl, Michael J wrote:

> >-----Original Message-----
> >From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> >Sent: Wednesday, June 11, 2025 6:42 AM
> >To: Ruhl, Michael J <michael.j.ruhl@intel.com>
> >Cc: platform-driver-x86@vger.kernel.org; intel-xe@lists.freedesktop.org; Hans
> >de Goede <hdegoede@redhat.com>; De Marchi, Lucas
> ><lucas.demarchi@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>;
> >thomas.hellstrom@linux.intel.com; airlied@gmail.com; simona@ffwll.ch;
> >david.e.box@linux.intel.com; stable@vger.kernel.org
> >Subject: Re: [PATCH v4 01/10] platform/x86/intel/pmt: fix a crashlog NULL
> >pointer access
> >
> >On Tue, 10 Jun 2025, Michael J. Ruhl wrote:
> >
> >> Usage of the intel_pmt_read() for binary sysfs, requires a pcidev.  The
> >> current use of the endpoint value is only valid for telemetry endpoint
> >> usage.
> >>
> >> Without the ep, the crashlog usage causes the following NULL pointer
> >> exception:
> >>
> >> BUG: kernel NULL pointer dereference, address: 0000000000000000
> >> Oops: Oops: 0000 [#1] SMP NOPTI
> >> RIP: 0010:intel_pmt_read+0x3b/0x70 [pmt_class]
> >> Code:
> >> Call Trace:
> >>  <TASK>
> >>  ? sysfs_kf_bin_read+0xc0/0xe0
> >>  kernfs_fop_read_iter+0xac/0x1a0
> >>  vfs_read+0x26d/0x350
> >>  ksys_read+0x6b/0xe0
> >>  __x64_sys_read+0x1d/0x30
> >>  x64_sys_call+0x1bc8/0x1d70
> >>  do_syscall_64+0x6d/0x110
> >>
> >> Augment the inte_pmt_entry to include the pcidev to allow for access to
> >
> >intel_pmt_entry
> 
> I have also been told that should be "intel_pmt_entry()"....  when I redo, is that
> more correct?

?? For structs, don't use (). Use () after any name that refers to a 
C function or a function like macro.

You could also say the struct intel_pmt_entry to indicate unambiguously to 
the reader what kind of object sits behind the name.

> Thanks,
> 
> M
> 
> >> the pcidev and avoid the NULL pointer exception.
> >>
> >> Fixes: 416eeb2e1fc7 ("platform/x86/intel/pmt: telemetry: Export API to read
> >telemetry")
> >> Cc: <stable@vger.kernel.org>
> >> Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> >> ---
> >>  drivers/platform/x86/intel/pmt/class.c | 3 ++-
> >>  drivers/platform/x86/intel/pmt/class.h | 1 +
> >>  2 files changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/platform/x86/intel/pmt/class.c
> >b/drivers/platform/x86/intel/pmt/class.c
> >> index 7233b654bbad..d046e8752173 100644
> >> --- a/drivers/platform/x86/intel/pmt/class.c
> >> +++ b/drivers/platform/x86/intel/pmt/class.c
> >> @@ -97,7 +97,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->pcidev, entry->cb, entry-
> >>header.guid, buf,
> >> +	count = pmt_telem_read_mmio(entry->pcidev, entry->cb, entry-
> >>header.guid, buf,
> >>  				    entry->base, off, count);
> >>
> >>  	return count;
> >> @@ -252,6 +252,7 @@ static int intel_pmt_populate_entry(struct
> >intel_pmt_entry *entry,
> >>  		return -EINVAL;
> >>  	}
> >>
> >> +	entry->pcidev = pci_dev;
> >>  	entry->guid = header->guid;
> >>  	entry->size = header->size;
> >>  	entry->cb = ivdev->priv_data;
> >> diff --git a/drivers/platform/x86/intel/pmt/class.h
> >b/drivers/platform/x86/intel/pmt/class.h
> >> index b2006d57779d..f6ce80c4e051 100644
> >> --- a/drivers/platform/x86/intel/pmt/class.h
> >> +++ b/drivers/platform/x86/intel/pmt/class.h
> >> @@ -39,6 +39,7 @@ struct intel_pmt_header {
> >>
> >>  struct intel_pmt_entry {
> >>  	struct telem_endpoint	*ep;
> >> +	struct pci_dev		*pcidev;
> >>  	struct intel_pmt_header	header;
> >>  	struct bin_attribute	pmt_bin_attr;
> >>  	struct kobject		*kobj;
> >>
> >
> >--
> > i.
> 

-- 
 i.