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: <87ce15c9-670e-f270-94b7-580b0a54f59f@amd.com>
Date:   Thu, 15 Jun 2023 09:05:12 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH] accel/qaic: Call DRM helper function to destroy prime GEM
Content-Language: en-US
To:     Jeffrey Hugo <quic_jhugo@quicinc.com>, quic_carlv@quicinc.com,
        quic_pkanojiy@quicinc.com, sukrut.bellary@linux.com,
        sumit.semwal@linaro.org
Cc:     ogabbay@kernel.org, linux-arm-msm@vger.kernel.org,
        kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linaro-mm-sig@lists.linaro.org,
        linux-media@vger.kernel.org
References: <20230614161528.11710-1-quic_jhugo@quicinc.com>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20230614161528.11710-1-quic_jhugo@quicinc.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?VWV1bklYSFZ1M3pxbjJBRElLbkNpNUpCazZTVDNTb2I1TmRqQTVlZEpOWHRE?=
 =?utf-8?B?bXU4K3NobVc4QXcweS81R0JMZFdNLzArL2JROU5IYzhVRUFwY2Z1SDBtdmxG?=
 =?utf-8?B?N0luZ1V4OGE2Y3g3R25PTjFvNGxFUkcxSm5IWDhpSG1QVEdTUXpPMFYvZExB?=
 =?utf-8?B?Ump1QVBjZzZQa0FXZi9icmFIWVg0MWh0MVdpZUFsK3c5YWd0NzF5MENuUGVy?=
 =?utf-8?B?SUF6Qit3dDIxOE1hd1RNTkdvNExMSFBrQitTZzlPZkNTdndKN3BtUjk4cFdO?=
 =?utf-8?B?QlE0YjZjbVlpeVl4U1BTdlc5MXN5YmE3OGF6ZnQvSGEybVptWWxiVTV6djMx?=
 =?utf-8?B?bFNTMlRrM3Jtbkk3ZzlsQStZM3JNaVdwc2tjZ0N1YXJMVzZBakVTNkF3MEcr?=
 =?utf-8?B?V0xyK00zeldZTzVNOFA4bHRHWDJQUkljc0t5OW1FdkdyTngrS1RybWYwNUZh?=
 =?utf-8?B?Qmt6RlZodmtWc3BJVE9NY3h2YmRtNmhucHQzZ1pNZ1NJN3EzcVg0eGM3OW9L?=
 =?utf-8?B?bUhud3JuSGtGL1l1aU5KeG9EbWNJTFRYcmN5SDNLSVpCN0hxdWQyTDRHRnhl?=
 =?utf-8?B?elBoODVVQUIvR1p1UEQ2REwzZjAxdi8vMGVNSE1lMlE4bk5rNnVQOHRMTW1E?=
 =?utf-8?B?ZXFnS3ZUWVlKWUZwbUZzV3lmZUNjaHNBRlRRRlR0N2dGUE9nT3VKVG9rcWt3?=
 =?utf-8?B?UUtVcExXbWtuZkYreENkUkUzbGg5OUl0M0pMeDBEUGtmbWp5enlERzh6VkN6?=
 =?utf-8?B?bWlQanBCd1hUQ0o1VUY2U0sxZW1NdXlMWStjY0dUblkrWHJGTTBCdVhxMFpH?=
 =?utf-8?B?eTY5NHNhenFvVkdpY3FQT3NOV3JtZEd0QnJVMFNlK1RlWWxkb2RhYXlVUGhX?=
 =?utf-8?B?UnFHZlkrMmNFNys2TVVrZHd4U2lWSVNndVU1YXlLSCtqajdSUGRKN29TZFY5?=
 =?utf-8?B?d3IvejN6c24ydXB5dXh6K2xRUlFBb3VQQVJkNUVaUXpQM0dlVVJZWTZKL2Zt?=
 =?utf-8?B?L09KZm1PUUR2MVFtRUZFRHF3UTlPOUltZWFDUThVejVLOUg5T09SRlMyQkNh?=
 =?utf-8?B?Y1p0bndUdE1yV3hPeVVPUFV2TkZ0a0JOcDZDb2lZM0ZEWE5qaCticVVLODRG?=
 =?utf-8?B?UkxvMXN6Uk1mLzdITEhWZzdydEIyMG4xTlV1TDE0clVDTGRpOWpRWW9tUHF1?=
 =?utf-8?B?QXBDMHZybC9pbnNUVXozaklQZE91cDlSdkcwb1lUN2E1UnNNTHA5Z1lFNDFR?=
 =?utf-8?B?MEtMaTh3YkQvenE1bXorbkZlUjVCaFplYUlzTXFhVVN5VXlHQ2F4cmdzcXg4?=
 =?utf-8?B?SjhGZ2EwOGlRL1NjVjZLejd5TU5BdzVzSWMvdHJMY0UrVjYxZUJqTk1tWjR5?=
 =?utf-8?B?bElRQVVkV3pSQnkxdG80MUkzUnNibXBuZzBWQUYvMVZJMWxKUGNwQ2dwNkJp?=
 =?utf-8?B?MXllVUhhajV3eW1vdVdPakpsOG91aFBJeTZIN2xBVUxBOHNLcWZFNDBKdUpn?=
 =?utf-8?B?cnROUTRmVXBadlhtUm0rNXNtSDhpRWJSVjE5WkFleDRkU05RYUJyZXFhMzFs?=
 =?utf-8?B?YnZ1eldTTHdZVWRUcDRHRUdvajc3T2hKREN4ODBHck5Wam5pWEhWNXlESmJ3?=
 =?utf-8?B?ZmIwTXYwTlZvNkdKaFBSKzNPMXpNZU9uVXBUZWt3K1pzUENhb3dkSFU5SzZR?=
 =?utf-8?B?MUFWWXQrbmZOeUt0ZmNESUUyVjRGYlhkRHloMkJ5emIzUnp0Vy9OeFlHQnp4?=
 =?utf-8?B?N1gxZ2x0MjZzTmVab3B1MGlDOXNOeXl5TndzWjBLcHdOanFzeTMvUWhEM0M2?=
 =?utf-8?B?a1BPMWNRTmpvOERacXJGamJxUWJTME5TdGNYclpKRnVvM0JCejhYYTVPVXNk?=
 =?utf-8?B?QlpsOWg5SkF3a25xZHlHbHN2N0t2RnVISU1ZNUs0UDFyQVdRVEZRVHdWNmty?=
 =?utf-8?B?U0JrekdyajJCaFA5blFOeTBoZkI4NzcrUllraWxYMXFyNllMU1RuYnRZWlJn?=
 =?utf-8?B?Q3FReEROUm5UV3JBWFo1VWtsdVkzSS9lU3VFeG9VM01ZYVA4MHl1UnBodlVI?=
 =?utf-8?B?aU5PaVk3ZVQycTYxTFRFVFNDazBYVXR3bnF4TWdwbHc3dzBXT09pMGRRaytD?=
 =?utf-8?B?ZVdqUW9OT21PdEo3YkI0a3Y1MHoxcmJVWG1NVEVFUDFvK29RUFcwNHYxaHhB?=
 =?utf-8?Q?HdLU/iQCVuKaW76DfW/wPb0/HDIRzPloUZkfJUDGc1ml?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 41f0e3b4-6e74-489e-724f-08db6d6ee15e
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3589.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Jun 2023 07:05:20.0801
 (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: yGQUJFbzoSJHvnXC7Y71WpmleeYvEHFWrLgNYgyZfCGwmCuZkesbxtTO5yiaKivk
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR12MB8255
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 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 14.06.23 um 18:15 schrieb Jeffrey Hugo:
> From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
>
> smatch warning:
> 	drivers/accel/qaic/qaic_data.c:620 qaic_free_object() error:
> 		dereferencing freed memory 'obj->import_attach'
>
> obj->import_attach is detached and freed using dma_buf_detach().
> But used after free to decrease the dmabuf ref count using
> dma_buf_put().
>
> drm_prime_gem_destroy() handles this issue and performs the proper clean
> up instead of open coding it in the driver.
>
> Fixes: ff13be830333 ("accel/qaic: Add datapath")
> Reported-by: Sukrut Bellary <sukrut.bellary@linux.com>
> Closes: https://lore.kernel.org/all/20230610021200.377452-1-sukrut.bellary@linux.com/
> Suggested-by: Christian König <christian.koenig@amd.com>
> Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@quicinc.com>
> Reviewed-by: Carl Vanderlip <quic_carlv@quicinc.com>
> Reviewed-by: Jeffrey Hugo <quic_jhugo@quicinc.com>
> Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/accel/qaic/qaic_data.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/accel/qaic/qaic_data.c b/drivers/accel/qaic/qaic_data.c
> index e42c1f9ffff8..e9a1cb779b30 100644
> --- a/drivers/accel/qaic/qaic_data.c
> +++ b/drivers/accel/qaic/qaic_data.c
> @@ -23,6 +23,7 @@
>   #include <linux/wait.h>
>   #include <drm/drm_file.h>
>   #include <drm/drm_gem.h>
> +#include <drm/drm_prime.h>
>   #include <drm/drm_print.h>
>   #include <uapi/drm/qaic_accel.h>
>   
> @@ -616,8 +617,7 @@ static void qaic_free_object(struct drm_gem_object *obj)
>   
>   	if (obj->import_attach) {
>   		/* DMABUF/PRIME Path */
> -		dma_buf_detach(obj->import_attach->dmabuf, obj->import_attach);
> -		dma_buf_put(obj->import_attach->dmabuf);
> +		drm_prime_gem_destroy(obj, NULL);
>   	} else {
>   		/* Private buffer allocation path */
>   		qaic_free_sgt(bo->sgt);