[AMD Official Use Only - AMD Internal Distribution Only] Hi Christian and Felix
Thanks for your comment, we will ignore these similar Coverity warnings. Regards Jesse -----Original Message----- From: Koenig, Christian <[email protected]> Sent: Thursday, May 30, 2024 10:11 PM To: Zhang, Jesse(Jie) <[email protected]>; [email protected] Cc: Deucher, Alexander <[email protected]>; Kuehling, Felix <[email protected]>; Kim, Jonathan <[email protected]>; Huang, Tim <[email protected]> Subject: Re: [PATCH 4/8] amd/amdkfd:fix overflowed constant in the function svm_migrate_copy_to_ram Am 30.05.24 um 05:48 schrieb Jesse Zhang: > If the svm migration copy memory gart fails or the dma mapping page fails for > the first time. > But the variable i is still 0, and executing i-- will overflow. > > Signed-off-by: Jesse Zhang <[email protected]> > --- > drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > index 8ee3d07ffbdf..3620eabf13c7 100644 > --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c > @@ -650,9 +650,10 @@ svm_migrate_copy_to_ram(struct amdgpu_device *adev, > struct svm_range *prange, > out_oom: > if (r) { > pr_debug("failed %d copy to ram\n", r); > - while (i--) { > + while (i) { > svm_migrate_put_sys_page(dst[i]); > migrate->dst[i] = 0; > + i--; That looks incorrect to me. "i" is usually the entry which failed and doesn't need to cleanup. So using "while (i---) ...." is a very common and correct way to clean things up. With the code changed as above 0 for example would never be cleaned up. Christian. > } > } >
