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

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

[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.

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;
-- 
2.39.5

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

[Christian König]

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;