| Message ID | 20241129132825.569237-1-ming.li@zohomail.com |
|---|---|
| State | Accepted |
| Commit | 09ceba3a93450b652ae6910b6f65be99885f4437 |
| Headers | show |
| Series | [1/1] cxl/pci: Check dport->regs.rcd_pcie_cap availability before accessing | expand |
On Fri, Nov 29, 2024 at 09:28:25PM +0800, Li Ming wrote: > RCD Upstream Port's PCI Express Capability is a component registers > block stored in RCD Upstream Port RCRB. CXL PCI driver helps to map it > during the RCD probing, but mapping failure is allowed for component > registers blocks in CXL PCI driver. > > dport->regs.rcd_pcie_cap is used to store the virtual address of the RCD > Upstream Port's PCI Express Capability, add a dport->regs.rcd_pcie_cap > checking in rcd_pcie_cap_emit() just in case user accesses a invalid > address via RCD sysfs. I traced this enough to convince myself that dport cannot be NULL if cxl_mem_find_port() succeeds. So, LGTM - Reviewed-by: Alison Schofield <alison.schofield@intel.com> > > Fixes: c5eaec79fa43 ("cxl/pci: Add sysfs attribute for CXL 1.1 device link status") > Signed-off-by: Li Ming <ming.li@zohomail.com> > --- > drivers/cxl/pci.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c > index b2cb81f6d9e7..e53b1c95a248 100644 > --- a/drivers/cxl/pci.c > +++ b/drivers/cxl/pci.c > @@ -836,6 +836,9 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size > if (!root_dev) > return -ENXIO; > > + if (!dport->regs.rcd_pcie_cap) > + return -ENXIO; > + > guard(device)(root_dev); > if (!root_dev->driver) > return -ENXIO; > -- > 2.34.1 >
Li Ming wrote: > RCD Upstream Port's PCI Express Capability is a component registers > block stored in RCD Upstream Port RCRB. CXL PCI driver helps to map it > during the RCD probing, but mapping failure is allowed for component > registers blocks in CXL PCI driver. > > dport->regs.rcd_pcie_cap is used to store the virtual address of the RCD > Upstream Port's PCI Express Capability, add a dport->regs.rcd_pcie_cap > checking in rcd_pcie_cap_emit() just in case user accesses a invalid > address via RCD sysfs. > > Fixes: c5eaec79fa43 ("cxl/pci: Add sysfs attribute for CXL 1.1 device link status") > Signed-off-by: Li Ming <ming.li@zohomail.com> Hi Ming, This patch looks ok. Reviewed-by: Dan Williams <dan.j.williams@intel.com> ...but it bothers me that the sysfs attributes are visible while the attributes are in this -ENXIO return state. I will throw together a follow-on patch to hide the attributes altogether when these preconditions are not met.
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index b2cb81f6d9e7..e53b1c95a248 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -836,6 +836,9 @@ static ssize_t rcd_pcie_cap_emit(struct device *dev, u16 offset, char *buf, size if (!root_dev) return -ENXIO; + if (!dport->regs.rcd_pcie_cap) + return -ENXIO; + guard(device)(root_dev); if (!root_dev->driver) return -ENXIO;
RCD Upstream Port's PCI Express Capability is a component registers block stored in RCD Upstream Port RCRB. CXL PCI driver helps to map it during the RCD probing, but mapping failure is allowed for component registers blocks in CXL PCI driver. dport->regs.rcd_pcie_cap is used to store the virtual address of the RCD Upstream Port's PCI Express Capability, add a dport->regs.rcd_pcie_cap checking in rcd_pcie_cap_emit() just in case user accesses a invalid address via RCD sysfs. Fixes: c5eaec79fa43 ("cxl/pci: Add sysfs attribute for CXL 1.1 device link status") Signed-off-by: Li Ming <ming.li@zohomail.com> --- drivers/cxl/pci.c | 3 +++ 1 file changed, 3 insertions(+)