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: <56f6f771-8ccc-ec91-e4dc-f4841bce38c5@amd.com>
Date:   Wed, 14 Jun 2023 11:11:30 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] drm/amdkfd: Switch over to memdup_user()
To:     Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc:     alexander.deucher@amd.com, christian.koenig@amd.com,
        Xinhui.Pan@amd.com, airlied@gmail.com, daniel@ffwll.ch,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, Abaci Robot <abaci@linux.alibaba.com>
References: <20230614020432.44044-1-jiapeng.chong@linux.alibaba.com>
Content-Language: en-US
From:   Felix Kuehling <felix.kuehling@amd.com>
In-Reply-To: <20230614020432.44044-1-jiapeng.chong@linux.alibaba.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SjJtZHp4OGpyajB0cC9oVElyMm55bXVycFJUbDRrSDR5OUIzemEwTlErUkVt?=
 =?utf-8?B?RmYyUW5XUGRaeW1WUlNOcjREMDNMM2U0MmJMWWhqL1BLTkdGb2ZDSkJHblpy?=
 =?utf-8?B?bTFqc0drVkV3c0xmSlBackhIRTUyTk9IWkZrcTh0RmdEK2JqTEpLanRNNnpj?=
 =?utf-8?B?VkhZWTFXd1ozcGhqMkRJYTBVSG1UV2Q1VnA1WDFTSk9nOTJ0RVhjdEIrUnp4?=
 =?utf-8?B?d0gza2VIR0FFM3I4OXZIaXQvNlBDZTJuQ0hSY1BaK29mc09QYVIrSzArd1cx?=
 =?utf-8?B?SWI5N1NwMW5vZ1dlYnd6R0NNNzRSdzEvMktyNFU1SncyU1NibUsvNmNjcC82?=
 =?utf-8?B?WDBVanBWZFlzNlhaR3BlM3ZNZnpKNGVRK2JoTmpTbStTZDFGcGN2ZUk4cWRn?=
 =?utf-8?B?c3lRU1I3K0ZqUVA1aUU2Mzk2eTVwY3l0VzJGeDczUkdieWo2Mm01K2pHOWlp?=
 =?utf-8?B?ckp3TldBWUN0emxmbkMrTTlLWlpENWhqb29HSUREWXRHRmZvKzgvZjJFSFM1?=
 =?utf-8?B?c2ZlWHN3STcrdXdRK3pXdE5pSjFvRHVHL1FaZWVrYkthOTlPdVdhQS9LcFVu?=
 =?utf-8?B?UVlmdzU4K2MraDREdkVCdEZJbWc5UUZQU0pFYVp1S2R5VFdUa1VoM0EzWkEz?=
 =?utf-8?B?R2J2S1hUK2JrVHRzR1NOT2FXcWczdCtVNWxSMTZ1b2YyMDR1Zmc4M0lxNEkw?=
 =?utf-8?B?SEhUWEt3SThoc25oZndhOUh2N3FWVE9NQlNFK29tTkV2V09jRlhjMlRWc2Ru?=
 =?utf-8?B?dmhFKzRUVEZTU1lUTldiYUQrN3ZwR1VkVDZESlU5bm5xTjdwWHgzK3Q2cys1?=
 =?utf-8?B?UksyZ2hiV1Z5VDhCOEt0RytuMlFoc0NjR20vQ2Q3SlJzSy8xNmFUb0JhYWV3?=
 =?utf-8?B?bzlCTXVsZUE3MXBOM3Q3SkhNMmFkc0Njc3ptYmc1cC9ueCs4YW9SSk9lcVFU?=
 =?utf-8?B?ZVJNV0pHOE9LcnFrUFdFdE8rdU1wZk1yMWhiYlZxL0ZxU2Zyd0FESEU1T2Jq?=
 =?utf-8?B?Um5oRFRtdkFXamhqbzBZQWZYWEpSWWgxQ0lLNHpRMWtvSFFrNnp2RngzSFg0?=
 =?utf-8?B?dUIxQnE0MlJ3eUwweW5uL0FCSk5WSkZpRzlvSTJTVnZXZnMwaWtLajdySlNV?=
 =?utf-8?B?aWlSL3JqeDVJZ3RMOWQ4aEFtY2I1TWsvRHVCcGQrdUk4Z0Y2ZlRPLzNvUHNX?=
 =?utf-8?B?RGhMV0FPYk4yaXkyYUMwMnhGcGl6V0xWNXUzcmFXVWdwcGhZc2FNUndNOExS?=
 =?utf-8?B?THpMNUNIREp1WUxQay9SMzNWZEZwT2FCd1BKNm9BRHpGUWQ1cGc1ZEdiTitE?=
 =?utf-8?B?MUtnSG9ERm5hS0ttV1RpcTRRVWVCRnVYQ0V6VGVVUEt3d3AvWHc5Ly9WdHpD?=
 =?utf-8?B?STRMR1ZrN2FIRitkSFQza0ZvRlY1Ynp4Wk03TmZKSml0U3o5cktCblZ0WWRm?=
 =?utf-8?B?WDlwRnYyWE9TU2dUNEQwb1dvL0ZEN0RCSzRoeFRPcDhZdjdjQ0Jtb0M2cGtm?=
 =?utf-8?B?cXlCM3BsNmVic1dqbWl2SElPNmI1bUVuLytJN1pkVzgyeXlOWUUyb2t2dlJQ?=
 =?utf-8?B?Sy80YklINUEvNjZidG9kTEJpTlFEMzkyM25ma0VSVWhQWUR1cGZWT3pKWUQv?=
 =?utf-8?B?R0NvakMxczhaR3hqekY1MG9RMmZqSDZkTXE3SlRDVWhEU3NNVXdqZUJhYysz?=
 =?utf-8?B?dG00RXBaYXBGbmMzcEFKRW9PL1F1SDZ3Sk52S3lyVXpVQzNUZnhJSTZ0Y3pJ?=
 =?utf-8?B?eFpGdFRDUkNseFJxc2JsZ0hIL0F6QWJuM3dud05DZVhLNU9LWW9xRnh4RE1u?=
 =?utf-8?B?WmRQZXY1Y1BmR3ZTVjlUcHpHc0tFRkJWVno4RGJocmhwSVpwUVNaTVF1WGlL?=
 =?utf-8?B?NWx5VUFXM2N0QXZGcGZqemdtbnp5REZqamt1REEwU3lrdkJtUXBaZnJGWjVw?=
 =?utf-8?B?cWN4OVBxS3lqdk0rZnBWclFyaUphc0tJTTk2cFdseVZ2eFVobWZNd0Z3QkEy?=
 =?utf-8?B?VjF6c3lNb2lORVNVcHhseXowVEFmaExScnJLeFM2WnhNZXBWNWd3WEEyT3lK?=
 =?utf-8?B?S2FEamQyZVduQ2lnRG1qa2VqM2R3eE5XWW9EcS9sVFhYMVlOMmRPTGlUZThh?=
 =?utf-8?Q?AGrIa9xA+w+Eu3u9051Yvb6yk?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 15aee1a9-8f3c-4f66-1f9a-08db6ce9a34c
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5115.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2023 15:11:32.7136
 (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: HrgoiO4WyQxhVorFeJyYJ5WqU4eQf84X3dG3bqXhg1jkNUECYVjnpA/wUHzHUHTagUHo3Ta/6XaKRIyRiweL2w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR12MB6570
X-Spam-Status: No, score=-1.2 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=no 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 2023-06-13 um 22:04 schrieb Jiapeng Chong:
> Use memdup_user() rather than duplicating its implementation. This is a
> little bit restricted to reduce false positives.
>
> ./drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c:2813:13-20: WARNING opportunity for memdup_user.
>
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=5523
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

Kernel test robot is reporting a failure with this patch, looks like you 
used PTR_ERR incorrectly. Please make sure your patch compiles without 
warnings.

I see more opportunities to use memdup_user in kfd_chardev.c, 
kfd_events.c, kfd_process_queue_manager.c and kfd_svm.c. Do you want to 
fix those, too, while you're at it?

Thanks,
   Felix


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> index d6b15493fffd..637962d4083c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_device_queue_manager.c
> @@ -2810,12 +2810,9 @@ static uint32_t *get_queue_ids(uint32_t num_queues, uint32_t *usr_queue_id_array
>   	if (!usr_queue_id_array)
>   		return NULL;
>   
> -	queue_ids = kzalloc(array_size, GFP_KERNEL);
> -	if (!queue_ids)
> -		return ERR_PTR(-ENOMEM);
> -
> -	if (copy_from_user(queue_ids, usr_queue_id_array, array_size))
> -		return ERR_PTR(-EFAULT);
> +	queue_ids = memdup_user(usr_queue_id_array, array_size);
> +	if (IS_ERR(queue_ids))
> +		return PTR_ERR(queue_ids);
>   
>   	return queue_ids;
>   }