On 10/23/2025 11:05 AM, Srinivasan Shanmugam wrote:
The function amdgpu_virt_get_dynamic_data_info() writes a 64-bit size
value. In two places (amdgpu_bios.c and amdgpu_discovery.c), the code
passed the address of a smaller variable by casting it to u64 *, which
is unsafe.
This could make the function write more bytes than the smaller variable
can hold, possibly overwriting nearby memory. Reported by static
analysis tools.
v2: Dynamic region size comes from the host (SR-IOV setup) and is always
fixed to 5 MB. (Lijo/Ellen)
5 MB easily fits inside a 32-bit value, so using a 64-bit type is not
needed. It also avoids extra type casts
Fixes: ae92010fb321 ("drm/amdgpu: Add logic for VF ipd and VF bios to init from
dynamic crit_region offsets")
Reported by: Dan Carpenter <[email protected]>
Cc: Ellen Pan <[email protected]>
Cc: Christian König <[email protected]>
Cc: Alex Deucher <[email protected]>
Signed-off-by: Srinivasan Shanmugam <[email protected]>
Reviewed-by: Lijo Lazar <[email protected]>
Thanks,
Lijo
---
drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c | 4 ++--
drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
index db705bf723f1..35d04e69aec0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c
@@ -103,7 +103,7 @@ static bool amdgpu_read_bios_from_vram(struct amdgpu_device
*adev)
{
uint8_t __iomem *bios = NULL;
resource_size_t vram_base;
- resource_size_t size = 256 * 1024; /* ??? */
+ u32 size = 256U * 1024U; /* ??? */
if (!(adev->flags & AMD_IS_APU))
if (amdgpu_device_need_post(adev))
@@ -126,7 +126,7 @@ static bool amdgpu_read_bios_from_vram(struct amdgpu_device
*adev)
*/
if (amdgpu_sriov_vf(adev) && adev->virt.is_dynamic_crit_regn_enabled) {
if (amdgpu_virt_get_dynamic_data_info(adev,
- AMD_SRIOV_MSG_VBIOS_IMG_TABLE_ID, adev->bios,
(uint64_t *)&size)) {
+ AMD_SRIOV_MSG_VBIOS_IMG_TABLE_ID, adev->bios,
&size)) {
amdgpu_bios_release(adev);
return false;
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
index a7cb4665f485..fa2a22dfa048 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_discovery.c
@@ -311,7 +311,7 @@ static int amdgpu_discovery_read_binary_from_mem(struct
amdgpu_device *adev,
*/
if (amdgpu_virt_get_dynamic_data_info(adev,
AMD_SRIOV_MSG_IPD_TABLE_ID,
binary,
- (uint64_t
*)&adev->discovery.size)) {
+ &adev->discovery.size)) {
dev_err(adev->dev,
"failed to read discovery info from
dynamic critical region.");
ret = -EINVAL;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
index 66e9cd103597..38cc446500d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
@@ -1101,7 +1101,7 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device
*adev)
}
int amdgpu_virt_get_dynamic_data_info(struct amdgpu_device *adev,
- int data_id, uint8_t *binary, uint64_t *size)
+ int data_id, uint8_t *binary, u32 *size)
{
uint32_t data_offset = 0;
uint32_t data_size = 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
index 2a13cc892a13..14d864be5800 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
@@ -443,7 +443,7 @@ void amdgpu_virt_init(struct amdgpu_device *adev);
int amdgpu_virt_init_critical_region(struct amdgpu_device *adev);
int amdgpu_virt_get_dynamic_data_info(struct amdgpu_device *adev,
- int data_id, uint8_t *binary, uint64_t *size);
+ int data_id, uint8_t *binary, u32 *size);
bool amdgpu_virt_can_access_debugfs(struct amdgpu_device *adev);
int amdgpu_virt_enable_access_debugfs(struct amdgpu_device *adev);
