[PATCH 4/8] amd/amdkfd:fix overflowed constant in the function svm_migrate_copy_to_ram

[Zhang, Jesse(Jie)]

[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 <Christian.Koenig at amd.com>
Sent: Thursday, May 30, 2024 10:11 PM
To: Zhang, Jesse(Jie) <Jesse.Zhang at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher at amd.com>; Kuehling, Felix <Felix.Kuehling at amd.com>; Kim, Jonathan <Jonathan.Kim at amd.com>; Huang, Tim <Tim.Huang at amd.com>
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 <Jesse.Zhang at amd.com>
> ---
>   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.

>               }
>       }
>