[Public]
Pls ignore v2,
-----Original Message-----
From: SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>
Sent: Monday, January 20, 2025 6:42 PM
To: Koenig, Christian <Christian.Koenig@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; SHANMUGAM, SRINIVASAN <SRINIVASAN.SHANMUGAM@xxxxxxx>; Dan Carpenter <dan.carpenter@xxxxxxxxxx>; Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Subject: [PATCH v2] drm/amd/amdgpu: Prevent null pointer dereference in GPU bandwidth calculation
If the parent is NULL, adev->pdev is used to retrieve the PCIe speed and width, ensuring that the function can still determine these capabilities from the device itself.
Fixes the below:
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:6193 amdgpu_device_gpu_bandwidth()
error: we previously assumed 'parent' could be null (see line 6180)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
6170 static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
6171 enum pci_bus_speed *speed,
6172 enum pcie_link_width *width)
6173 {
6174 struct pci_dev *parent = adev->pdev;
6175
6176 if (!speed || !width)
6177 return;
6178
6179 parent = pci_upstream_bridge(parent);
6180 if (parent && parent->vendor == PCI_VENDOR_ID_ATI) {
^^^^^^
If parent is NULL
6181 /* use the upstream/downstream switches internal to dGPU */
6182 *speed = pcie_get_speed_cap(parent);
6183 *width = pcie_get_width_cap(parent);
6184 while ((parent = pci_upstream_bridge(parent))) {
6185 if (parent->vendor == PCI_VENDOR_ID_ATI) {
6186 /* use the upstream/downstream switches internal to dGPU */
6187 *speed = pcie_get_speed_cap(parent);
6188 *width = pcie_get_width_cap(parent);
6189 }
6190 }
6191 } else {
6192 /* use the device itself */
--> 6193 *speed = pcie_get_speed_cap(parent);
^^^^^^ Then we are toasted here.
6194 *width = pcie_get_width_cap(parent);
6195 }
6196 }
Fixes: 9e424a5d9087 ("drm/amdgpu: cache gpu pcie link width")
Cc: Christian König <christian.koenig@xxxxxxx>
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@xxxxxxx>
Suggested-by: Lijo Lazar <lijo.lazar@xxxxxxx>
---
v2: Use the device itself if no upstream bridge is found (Lijo)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 46af07faf8c8..3df5247b1221 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6173,8 +6173,13 @@ static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
if (!speed || !width)
return;
+ if (!parent) {
+ /* Use the device itself if no upstream bridge is found */
+ parent = adev->pdev;
+ }
+
parent = pci_upstream_bridge(parent);
- if (parent && parent->vendor == PCI_VENDOR_ID_ATI) {
+ if (parent->vendor == PCI_VENDOR_ID_ATI) {
/* use the upstream/downstream switches internal to dGPU */
*speed = pcie_get_speed_cap(parent);
*width = pcie_get_width_cap(parent);
--
2.34.1
