Reviewed-by: Alex Deucher <[email protected]>
On Wed, Oct 22, 2025 at 2:36 PM Pan, Ellen <[email protected]> wrote: > > [AMD Official Use Only - AMD Internal Distribution Only] > > Please have Alex or Lijo reviewed it too. > > Reviewed-by: Ellen Pan <[email protected]> > > Thanks, > Ellen > > -----Original Message----- > From: SHANMUGAM, SRINIVASAN <[email protected]> > Sent: Wednesday, October 22, 2025 8:20 AM > To: Koenig, Christian <[email protected]>; Deucher, Alexander > <[email protected]> > Cc: [email protected]; SHANMUGAM, SRINIVASAN > <[email protected]>; Pan, Ellen <[email protected]> > Subject: [PATCH] drm/amdgpu: Make SR-IOV critical region checks overflow-safe > > The function amdgpu_virt_init_critical_region() contained an invalid check > for a negative init_hdr_offset value: > > if (init_hdr_offset < 0) > > Since init_hdr_offset is an unsigned 32-bit integer, this condition can never > be true and triggers a Smatch warning: > > warn: unsigned 'init_hdr_offset' is never less than zero > > In addition, the subsequent bounds check: if ((init_hdr_offset + > init_hdr_size) > vram_size) was vulnerable to integer overflow when adding > the two unsigned values. Thus, by promoting offset and size to 64-bit and > using check_add_overflow() to safely validate the sum against VRAM size. > > Fixes: a5d4d72eee03 ("drm/amdgpu: Introduce SRIOV critical regions v2 during > VF init") 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]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > index 66e9cd103597..45f2ad083338 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c > @@ -937,9 +937,10 @@ static uint8_t > amdgpu_virt_crit_region_calc_checksum(uint8_t *buf_start, uint8_t int > amdgpu_virt_init_critical_region(struct amdgpu_device *adev) { > struct amd_sriov_msg_init_data_header *init_data_hdr = NULL; > - uint32_t init_hdr_offset = adev->virt.init_data_header.offset; > - uint32_t init_hdr_size = adev->virt.init_data_header.size_kb << 10; > - uint64_t vram_size; > + u64 init_hdr_offset = adev->virt.init_data_header.offset; > + u64 init_hdr_size = (u64)adev->virt.init_data_header.size_kb << 10; > /* KB → bytes */ > + u64 vram_size; > + u64 end; > int r = 0; > uint8_t checksum = 0; > > @@ -957,7 +958,7 @@ int amdgpu_virt_init_critical_region(struct amdgpu_device > *adev) > return -EINVAL; > vram_size <<= 20; > > - if ((init_hdr_offset + init_hdr_size) > vram_size) { > + if (check_add_overflow(init_hdr_offset, init_hdr_size, &end) || end > > +vram_size) { > dev_err(adev->dev, "init_data_header exceeds VRAM size, > exiting\n"); > return -EINVAL; > } > -- > 2.34.1 >
