Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <5eae1b0d-a917-fc28-16ba-29bff612caac@amd.com>
Date:   Wed, 19 Apr 2023 10:38:25 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH V2 2/2] drm/amdgpu: Fix integer overflow in
 amdgpu_cs_pass1
Content-Language: en-US
To:     hackyzh002 <hackyzh002@gmail.com>, alexander.deucher@amd.com
Cc:     Xinhui.Pan@amd.com, airlied@gmail.com, daniel@ffwll.ch,
        sumit.semwal@linaro.org, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        linux-media@vger.kernel.org, linaro-mm-sig@lists.linaro.org
References: <20230419082705.4110-1-hackyzh002@gmail.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230419082705.4110-1-hackyzh002@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YXFXN1Z3VXZYZHIwN2c2RlpIVGtuNnVDeHNMMGdZZkJJQWpuM2pWN0NJeVRM?=
 =?utf-8?B?cWFFNENJTFpkTkhBZG9zMWVkQ2R5RFc1dnlLSTBjTkVWQ0NxZkFsN3E4UDB1?=
 =?utf-8?B?Tmx6WlZkYW0rU1RKbzkzYTBhVmdLSGdZSnJ4TTRybzcrRThhdzZSMnovNWRu?=
 =?utf-8?B?SFhSUkl6QzZUc3hsaTdCOWFhenZhZ3NuNE1CbmVodXVFajYzUDhNQzVKNWxL?=
 =?utf-8?B?bVh4d1lQTnAyNDJqSjQ5SzdtMXRESXJaTU1ZZW5nSzhWbEF0cHdEc0lzNit5?=
 =?utf-8?B?ZU5IWUdxQStUNHJoeWxvQzJNZ1dobUFFUG01Y2NVZFRvbzB6VWJuYkIycTQ2?=
 =?utf-8?B?YVh3MTJVNXhLOVo1TWpoTi9LTGV6dkl3a3FvVmN6aUpsRjlUNWY3V3c2aUQw?=
 =?utf-8?B?V0hVUFVxMDFNWkhmcFhGVlJ1UXc3SWEzMmVlblNXNW10amRoai9GWCt0Q1Zn?=
 =?utf-8?B?ZDF1cjh0QUsveEVoYXBlMDlxZEU5dFZzWnh4aTBVeWVjbXJnekh1OEw5dHhD?=
 =?utf-8?B?T2toZG0xZWh4dGhLWE5nV2hPOXkrLzVhRHYxNnV5d0FhRTNkSlNPQWk0bVJ3?=
 =?utf-8?B?WmJtQzY0VzU0c2M3QTUwejFvOCtrOVRJVTA5aWYycXFrenFqWkFCVHVzNG9v?=
 =?utf-8?B?b0YrVHVxdnBXN0FtMi83dVVrSmJRVDhuY0YvVlU0VmdhZ2VnMmxnZUxsc0dX?=
 =?utf-8?B?NEFIKy84UzBnQUcwa3VhMzNqR2hhN0VEOVcxcmxrR1lkMTRXa3RFM29pRFhl?=
 =?utf-8?B?bzRnMURxU0tHQ1R2VktZc3IzcS9rMFNPMzY2SnJYWkIyTmVpOUhzMUZEODhP?=
 =?utf-8?B?SXNjVUZpT0tSUWg2UTc1RjdGQ1pVaS9jMDZXcUMzamJrY0orYmtWZDlNUjRr?=
 =?utf-8?B?M3Y3c2JZeVltc2E3eFZGMHpENENFUzJWaHlpQkpWVGNWNklFSHpwUjdIbkN5?=
 =?utf-8?B?eGdDVWVjNFVoVStqVGxqU3pqKzVMcW0xV0tCMjZKd3V0RnVzbVQ3SWZDOXRq?=
 =?utf-8?B?c0VvNnJnY2V2Y3VYQ2RjZFZxUjhzY0NGTGdEbXVlcDVGS2lHK2w5SlcrbEtw?=
 =?utf-8?B?T2NPdEkzTDJMd04yYm05amZWbHpUOEpxeEZwSk13QVFYNzAzR2U4Z21kMlJP?=
 =?utf-8?B?TDc2dUFEbXAyT0N5Qlk2NEhJckxpRTVzZDg1MDhEdlBodEl3TUgzMTNOTzYy?=
 =?utf-8?B?OUhKTnd1SWFnMHpGQ25OY3hyNWVNSjhzK08vMWFjcmJvS1Z2OGdpM29LREhS?=
 =?utf-8?B?Z0xFUzROc29zRDRPKzRwZ0w5dmc5VDg1TWxHNkYreVB6Vlc4dEFzUHBFMm1V?=
 =?utf-8?B?d3BjYjN0VVdXazFNNlFaWkZxaEQ5SGNUUUd0ZDd6clZlUmE4ejZtRjVuU0pL?=
 =?utf-8?B?N200WWpBcDJ5aEl0UU1nNzlvWHg2OERKWDh5ZHA0V2hJSkdVcjR4LytpQWxS?=
 =?utf-8?B?cVViVmlRZGhIKzNpeFIrZWU0M0xGRFFVano0UU4yVVBhZGVZeUtadUU2R3lX?=
 =?utf-8?B?aGpobVlzeTg3MkFibTM5MjhycmpOLzR4MFZqckoyaUhZMERwQ2dLV1lKUlVl?=
 =?utf-8?B?aGtQa2tFSVFuNDcvYkNKWjA3b1I0eVpYNmVjMVhvL1BHYUtsQzRaOEwxT3l1?=
 =?utf-8?B?Z0p5bVFKcnlUVXllSVBUWm44VnNweGJWSGxoa2w5THVZUFNJYjI5NHNIcmNl?=
 =?utf-8?B?UVhrbEgyTzhlakk4UFVkdU5BQ2R4U3U0bnBOV0xkWWhkOFZZY2NVSFNkR05a?=
 =?utf-8?B?WmlwS0crMlpweEp3cjdHNWdPVHdyZldTV0xZdzdhS2FrQWtIQ1lmTWtwa3Jr?=
 =?utf-8?B?UmRpU0MvN1FoWGIvMjNvL0hLT0hLd0U3RDVyaUk4R1RPcW1wbFVueHlTZlhv?=
 =?utf-8?B?dWsyRHhnQmo4a1E2K0tCdHlXT1A4b253ZHF0OXUvL29ZRjlkWnJRNEtLdSsx?=
 =?utf-8?B?WXNrVEhFTDEra29WS2owVitob1dOaXRRcGpqWmVNRU9lRDZiaGlKUnVtM0ZH?=
 =?utf-8?B?V25PaWtUWmZvUFg2eGpzMzBwSG1nYWx2QkVMcTVBMFVTeHhOUmRaN1gxMXZk?=
 =?utf-8?B?WkZYbXNITUovZTdGd0d4azQ3ZnNuS3FSaUFlTmp2NUNaU2Jpd2kyUm1nWGxK?=
 =?utf-8?B?eUNYd2pqSE5JVC9MakloYVZyaUhhV3o0cXJ4ZG5WSVBoejlDQTZTTzd1RkNQ?=
 =?utf-8?Q?Ca3sU4G2FPQ3Yc5rR7d2Hw52AyjLAy7bQEVXm3+DWMg6?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4af3334f-7afd-47e8-aba2-08db40b1743d
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Apr 2023 08:38:30.9065
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Rv6Lmk51nwVmaJIyYwkPWICKKnAhCzdP9YiilO299FrRqfXamc9BLxJeI7Msdst6
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR12MB4427
X-Spam-Status: No, score=-3.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Am 19.04.23 um 10:27 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@gmail.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;
> +	size_t size;
>   	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),

Again, please drop that part of the patch. This is superfluous.

Regards,
Christian.

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