Re: [PATCH][next] drm/amdgpu: Fix potential integer overflow on shift of a int

["Christian König" <christian.koenig@amd.com>]

Am 10.10.24 um 13:22 schrieb Colin Ian King:
> The left shift of int 32 bit integer constant 1 is evaluated using 32 bit
> arithmetic and then assigned to and operated upon using a 64 bit unsigned
> integer. In the case where the shift is 32 or more this can lead to an
> overflow. Avoid this by shifting using the BIT_ULL macro instead.

Advait Dhamorikar already came up with a similar patch which also cleans 
up some other occasions.

Regards,
Christian.

>
> Fixes: f0b19b84d391 ("drm/amdgpu: add amdgpu_jpeg_sched_mask debugfs")
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
> index 95e2796919fc..136a0c8d8c7a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_jpeg.c
> @@ -357,14 +357,14 @@ static int amdgpu_debugfs_jpeg_sched_mask_set(void *data, u64 val)
>   	if (!adev)
>   		return -ENODEV;
>   
> -	mask = (1 << (adev->jpeg.num_jpeg_inst * adev->jpeg.num_jpeg_rings)) - 1;
> +	mask = (BIT_ULL(adev->jpeg.num_jpeg_inst * adev->jpeg.num_jpeg_rings)) - 1;
>   	if ((val & mask) == 0)
>   		return -EINVAL;
>   
>   	for (i = 0; i < adev->jpeg.num_jpeg_inst; ++i) {
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
>   			ring = &adev->jpeg.inst[i].ring_dec[j];
> -			if (val & (1 << ((i * adev->jpeg.num_jpeg_rings) + j)))
> +			if (val & BIT_ULL((i * adev->jpeg.num_jpeg_rings) + j))
>   				ring->sched.ready = true;
>   			else
>   				ring->sched.ready = false;
> @@ -388,7 +388,7 @@ static int amdgpu_debugfs_jpeg_sched_mask_get(void *data, u64 *val)
>   		for (j = 0; j < adev->jpeg.num_jpeg_rings; ++j) {
>   			ring = &adev->jpeg.inst[i].ring_dec[j];
>   			if (ring->sched.ready)
> -				mask |= 1 << ((i * adev->jpeg.num_jpeg_rings) + j);
> +				mask |= BIT_ULL((i * adev->jpeg.num_jpeg_rings) + j);
>   		}
>   	}
>   	*val = mask;