[PATCH 1/1] cxl/pci: Check dport->regs.rcd_pcie_cap availability before accessing

[PATCH 1/1] cxl/pci: Check dport->regs.rcd_pcie_cap availability before accessing

[Li Ming]

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(+)

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

Re: [PATCH 1/1] cxl/pci: Check dport->regs.rcd_pcie_cap availability before accessing

[Alison Schofield]

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
> 

Re: [PATCH 1/1] cxl/pci: Check dport->regs.rcd_pcie_cap availability before accessing

[Dan Williams]

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.