* [PATCH v3] drm/amd/amdgpu: Prevent null pointer dereference in GPU bandwidth calculation
@ 2025-01-20 13:31 Srinivasan Shanmugam
2025-01-20 13:33 ` Christian König
0 siblings, 1 reply; 2+ messages in thread
From: Srinivasan Shanmugam @ 2025-01-20 13:31 UTC (permalink / raw)
To: Christian König, Alex Deucher
Cc: amd-gfx, Srinivasan Shanmugam, Dan Carpenter, Lijo Lazar
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@amd.com>
Cc: Alex Deucher <alexander.deucher@amd.com>
Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Suggested-by: Lijo Lazar <lijo.lazar@amd.com>
---
v3: change the else s/parent/adev->pdev (Lijo)
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 46af07faf8c8..8ed7f2f8546d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6174,7 +6174,7 @@ static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
return;
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);
@@ -6187,8 +6187,8 @@ static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
}
} else {
/* use the device itself */
- *speed = pcie_get_speed_cap(parent);
- *width = pcie_get_width_cap(parent);
+ *speed = pcie_get_speed_cap(adev->pdev);
+ *width = pcie_get_width_cap(adev->pdev);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v3] drm/amd/amdgpu: Prevent null pointer dereference in GPU bandwidth calculation
2025-01-20 13:31 [PATCH v3] drm/amd/amdgpu: Prevent null pointer dereference in GPU bandwidth calculation Srinivasan Shanmugam
@ 2025-01-20 13:33 ` Christian König
0 siblings, 0 replies; 2+ messages in thread
From: Christian König @ 2025-01-20 13:33 UTC (permalink / raw)
To: Srinivasan Shanmugam, Alex Deucher; +Cc: amd-gfx, Dan Carpenter, Lijo Lazar
Am 20.01.25 um 14:31 schrieb Srinivasan Shanmugam:
> 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@amd.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> Suggested-by: Lijo Lazar <lijo.lazar@amd.com>
> ---
> v3: change the else s/parent/adev->pdev (Lijo)
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 46af07faf8c8..8ed7f2f8546d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -6174,7 +6174,7 @@ static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
> return;
>
> parent = pci_upstream_bridge(parent);
> - if (parent && parent->vendor == PCI_VENDOR_ID_ATI) {
> + if (parent->vendor == PCI_VENDOR_ID_ATI) {
That will now crash when parent is NULL :)
Christian.
> /* use the upstream/downstream switches internal to dGPU */
> *speed = pcie_get_speed_cap(parent);
> *width = pcie_get_width_cap(parent);
> @@ -6187,8 +6187,8 @@ static void amdgpu_device_gpu_bandwidth(struct amdgpu_device *adev,
> }
> } else {
> /* use the device itself */
> - *speed = pcie_get_speed_cap(parent);
> - *width = pcie_get_width_cap(parent);
> + *speed = pcie_get_speed_cap(adev->pdev);
> + *width = pcie_get_width_cap(adev->pdev);
> }
> }
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-01-20 13:33 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-20 13:31 [PATCH v3] drm/amd/amdgpu: Prevent null pointer dereference in GPU bandwidth calculation Srinivasan Shanmugam
2025-01-20 13:33 ` Christian König
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox