Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Message-ID: <b2bfdb5e-25d7-22d8-1788-ba34256e7664@amd.com>
Date:   Wed, 8 Jun 2022 10:35:13 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH 1/1] drm/radeon: Initialize fences array entries in
 radeon_sa_bo_next_hole
Content-Language: en-US
To:     Xiaohui Zhang <xiaohuizhang@ruc.edu.cn>,
        Alex Deucher <alexander.deucher@amd.com>, Xinhui.Pan@amd.com,
        David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20220607151933.32850-1-xiaohuizhang@ruc.edu.cn>
From:   =?UTF-8?Q?Christian_K=c3=b6nig?= <christian.koenig@amd.com>
In-Reply-To: <20220607151933.32850-1-xiaohuizhang@ruc.edu.cn>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 2
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M0NnMUF3bHQxWm1jVjErQ0JleVc2a0h4eW0wUXpMVFdvdDhpNGtDT0lvbnRU?=
 =?utf-8?B?cFUvdURnWmQveWJoOXJjczRldEVtdGV4cmYxL0ZhU1BJc1NFNXEwSXNjTFZz?=
 =?utf-8?B?dW1iQWcyRm9DUEdZYUt0clo0K1FKcDR2QkNiNVB0a1NrdjBoRjA3QWpod3po?=
 =?utf-8?B?VTVCeGhTejNCRmMrNzhpc3ZSYjV5Y1NuMUp2NDVuRTh6T2hwemQ5RnVFdDRh?=
 =?utf-8?B?RXlrOFgza1RaMEZBTUMycFdveG1INWlxbzFpeUdJYUJDdURDYmVneWo1MWdW?=
 =?utf-8?B?QmNaVkFSY0V0UUVJS012Zzd6dWV2dm5rM250eHZkR0lmUUsxNGJZalZndDYr?=
 =?utf-8?B?N3QrTUFPd2RNbFNMa2pwT2xOcDQwTlQzMXV3OURveVJxUWFNbEJEaWVMZFRD?=
 =?utf-8?B?M2ZrLzRvU3NYV1ZYV1NzZFVpVkZLQzZhYVRnUVVRNlJTdS9pUi9kbXRQQnBG?=
 =?utf-8?B?QVh0RmxzNmlTZFl3c1kyemx4NjV5aFh1TWNub3FHMHhVZkZzWll4dzBHUUp1?=
 =?utf-8?B?cGVYR3VjalRON2RuSGVQY2F3eE5taE9KRGxmNXV2NURJUWJIWFdJSUFVTkhB?=
 =?utf-8?B?OHVod1F6TTVJM0N3U1BVK01aZlZ4aUt5cGh6NWRpUTZOVjNPNUtSeDVOSFpE?=
 =?utf-8?B?M0VEcVEzTWRmdUZHKzBwQ01qK2V4OUVrUmV3SkdNT1NTRmdpejM2eTRzZ1RP?=
 =?utf-8?B?RjJmUk01RHBHRnJFamtsZmZjdzdZK0RhRVJyL0s0bFYrdWpFUm9HNlE2ZnNK?=
 =?utf-8?B?SFZtOVN5cmkzeUhwUFhaaGVScWEvOGE3OEZYd0ttV21DemViZEtMckFKQjZU?=
 =?utf-8?B?bVJ1bkNxU2J1N1FUTWhOWU4xNzMzUnJlVVU5TWNXZEhreXA3dldwd1lRK0Yx?=
 =?utf-8?B?S1A1UHlVM1V5UjVodU5kMjlpVWRNTDRzTW5IMFFWZFlqK0pVUWhlalk5N2NF?=
 =?utf-8?B?Z2xuZGF3K2NIckVBalo1QWdlMVhZM2hCcUdFZjdZYXFKY3FOT2hxQWRVWkp3?=
 =?utf-8?B?YmlrNnlNZ1M0Y0E2OUthY0FGNXNFZTg1ZzZINmM5L2FNTUVSU3pxa3BVQkow?=
 =?utf-8?B?dnVMYWRRQkpUbmVhU1BtRlF5eXg5Ri93b1hyVCtlZC83dkQ0TFowc0ZTeTBl?=
 =?utf-8?B?eXJyVFdEdkFHQW1ZOVFCbDJsRFdKVXgxQ2F4NXY3TC9NTCsvQlhvckNUUUxh?=
 =?utf-8?B?NTB5OUZQRWx5UWpPZ3BQZDQ2U0Vya3NDemJkbEc4d2ZYbWlwTFFxK3kvS0Yy?=
 =?utf-8?B?RVVCOXBuN3BBWHl0QjRQL296OTRVdjVkTUczVWhlTll1RjRBak9ZbmdNbEIz?=
 =?utf-8?B?VVgySVcwa2g4TEhBZVBpNUtpb2ZYa2NNdjNESFFVVGtDUlVvTGUvYkpvZnhH?=
 =?utf-8?B?R1JBSU9ob09BQlptQ3FDMFdsR1BMekFuMXJNZCsvV1VvOGhTMkg2UGpoak9Z?=
 =?utf-8?B?cmtxV3RrRWc3ZklFWkovbElGSHZFUXpsL0NYTGJGYkFqY2h0b2pzTnpqR0lJ?=
 =?utf-8?B?RnNEK1Nmd0VubzhVUmd1Qi9aZHYyTmdqZllLQ28yekNMTjVaaWhBL1duNTdw?=
 =?utf-8?B?OGkxM2YrSkxpREtkZ3hBUWhWdWZUMzRKNnYwWkxoNVI0VldtbkpCZVBrSVE5?=
 =?utf-8?B?dDlRaGlOa1l2enVpeTY1eW1SOExOR2VRVEJoTk02WHBhYlYwUXk5UXZCMHBh?=
 =?utf-8?B?RXdLenAvUkZCOGZkWDlJNW8xNWVPUWVWSUxqTllVaUdYRXowMXRabUlLRm5C?=
 =?utf-8?B?LzBIQ0Q4QXFaTnpuVU9XN2syQTQrZzAvbUw4N2NGQnhPUkk4MjV5bGFoSzBU?=
 =?utf-8?B?UmFsSG5idFVNNldaNDJqZVV4bTBUOFFwaWRNR29hY3B3aEFGVXpPSXpxY2tO?=
 =?utf-8?B?cnc3RnE1Q282aG02cVZKK0F4YXBMekNDQzVCSXc2OVd5VlJXRi9lMWxrZ1Fk?=
 =?utf-8?B?bnY2bWc1UDlSU1dnZk9YNGx2WTgyWloxMzNZdEU4NWFVUThDNHhERTEwQzhm?=
 =?utf-8?B?TEVvN0pobGZXcTZCQ3hzWElBRmtRazU4T3ZPbDVxR01aaitWV1hjeDhoR3JZ?=
 =?utf-8?B?SysvWnpqZ0dIU1poRUorNzhETzRvcWkrWW1WQWlwcnpUOGNlMW93ZCtZR01K?=
 =?utf-8?B?S1NHZ1QwTGxPNXdhUUZlWXJBakhSTU1UcWwwd2NDR3NqVnpkOE1LSTh0bTJG?=
 =?utf-8?B?REJoZGQ1YnVRTHliWU4yYlhZRmx6RFJKUFJXNll6bENZTzlHOG9hWUJTSGVR?=
 =?utf-8?B?cklUKy9qL012YlBPcU02M09COTM0Zm1uMkpHQmJQdkQ1ZHdjL0I1T0R4MVRq?=
 =?utf-8?B?UVZXRGlwdGs5T29LeXZYT3NQOXpIR0gxcHRMYmtkT1lnUTRXcEV4R2IwaHFQ?=
 =?utf-8?Q?Z4li4e6LfOHRkaMFtD7dYJ3FdqUvqWquGL32ldJIi04rU?=
X-MS-Exchange-AntiSpam-MessageData-1: p4mqBjnlsbvhng==
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7de2be80-9d48-4ee5-e86e-08da4929d1b6
X-MS-Exchange-CrossTenant-AuthSource: BN8PR12MB3587.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Jun 2022 08:35:18.9445
 (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: nfViJrY8L1P0DODqXu+TpfXPiQil4qojPInlN+bgvFrnfbs57/WnLqBkHY19cOTi
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR12MB4180
X-Spam-Status: No, score=-3.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable 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 07.06.22 um 17:19 schrieb Xiaohui Zhang:
> Similar to the handling of amdgpu_sa_bo_next_hole in commit 6a15f3ff19a8
> ("drm/amdgpu: Initialize fences array entries in amdgpu_sa_bo_next_hole"),
> we thought a patch might be needed here as well.
>
> The entries were only initialized once in radeon_sa_bo_new. If a fence
> wasn't signalled yet in the first radeon_sa_bo_next_hole call, but then
> got signalled before a later radeon_sa_bo_next_hole call, it could
> destroy the fence but leave its pointer in the array, resulting in
> use-after-free in radeon_sa_bo_new.

I would rather like to see the sub allocator moved into a common drm helper.

Regards,
Christian.

>
> Signed-off-by: Xiaohui Zhang <xiaohuizhang@ruc.edu.cn>
> ---
>   drivers/gpu/drm/radeon/radeon_sa.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_sa.c b/drivers/gpu/drm/radeon/radeon_sa.c
> index 310c322c7112..0981948bd9ed 100644
> --- a/drivers/gpu/drm/radeon/radeon_sa.c
> +++ b/drivers/gpu/drm/radeon/radeon_sa.c
> @@ -267,6 +267,8 @@ static bool radeon_sa_bo_next_hole(struct radeon_sa_manager *sa_manager,
>   	for (i = 0; i < RADEON_NUM_RINGS; ++i) {
>   		struct radeon_sa_bo *sa_bo;
>   
> +		fences[i] = NULL;
> +
>   		if (list_empty(&sa_manager->flist[i])) {
>   			continue;
>   		}
> @@ -332,10 +334,8 @@ int radeon_sa_bo_new(struct radeon_device *rdev,
>   
>   	spin_lock(&sa_manager->wq.lock);
>   	do {
> -		for (i = 0; i < RADEON_NUM_RINGS; ++i) {
> -			fences[i] = NULL;
> +		for (i = 0; i < RADEON_NUM_RINGS; ++i)
>   			tries[i] = 0;
> -		}
>   
>   		do {
>   			radeon_sa_bo_try_free(sa_manager);