linux-kernel - Re: [PATCH 2/2] drm/amdgpu: Fix integer overflow in amdgpu_cs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d2ecb01b-5895-da7e-60cf-787f9425115c@amd.com>
Date:   Wed, 19 Apr 2023 10:14:17 +0200
From:   Christian König <christian.koenig@....com>
To:     hackyzh002 <hackyzh002@...il.com>, alexander.deucher@....com
Cc:     Xinhui.Pan@....com, airlied@...il.com, daniel@...ll.ch,
        sumit.semwal@...aro.org, amd-gfx@...ts.freedesktop.org,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        linux-media@...r.kernel.org, linaro-mm-sig@...ts.linaro.org
Subject: Re: [PATCH 2/2] drm/amdgpu: Fix integer overflow in amdgpu_cs_pass1

Am 19.04.23 um 06:51 schrieb hackyzh002:
> The type of size is unsigned int, if size is 0x40000000, there will
> be an integer overflow, size will be zero after size *= sizeof(uint32_t),
> will cause uninitialized memory to be referenced later.
>
> Signed-off-by: hackyzh002 <hackyzh002@...il.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 08eced097..c17b3af85 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -192,7 +192,7 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
>   	uint64_t *chunk_array_user;
>   	uint64_t *chunk_array;
>   	uint32_t uf_offset = 0;
> -	unsigned int size;
> +	uint64_t int size;

This doesn't even seems to be compile tested. Please use size_t here.

>   	int ret;
>   	int i;
>   
> @@ -235,7 +235,7 @@ static int amdgpu_cs_pass1(struct amdgpu_cs_parser *p,
>   		size = p->chunks[i].length_dw;
>   		cdata = u64_to_user_ptr(user_chunk.chunk_data);
>   
> -		p->chunks[i].kdata = kvmalloc_array(size, sizeof(uint32_t),
> +		p->chunks[i].kdata = kvcalloc(size, sizeof(uint32_t),
>   						    GFP_KERNEL);

And that is unnecessary.

Regards,
Christian.



>   		if (p->chunks[i].kdata == NULL) {
>   			ret = -ENOMEM;