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: <9bce7374-ef67-75d0-4040-870ba131819e@amd.com>
Date:   Wed, 5 Apr 2023 14:23:32 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.9.1
Content-Language: en-CA
To:     Asahi Lina <lina@asahilina.net>, David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>
Cc:     dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        asahi@lists.linux.dev
References: <20230406-scheduler-uaf-2-v1-1-972531cf0a81@asahilina.net>
From:   Luben Tuikov <luben.tuikov@amd.com>
Subject: Re: [PATCH] drm/scheduler: Fix UAF race in
 drm_sched_entity_push_job()
In-Reply-To: <20230406-scheduler-uaf-2-v1-1-972531cf0a81@asahilina.net>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?SDFvMFJUNjFZRFFQWjI4SkNkYi9rdkUyYWpCdENJRUFHVU5WUVV1Tndic2Jp?=
 =?utf-8?B?cllnREE0My9QSDQxNXpvd0l1RFFkMFRvdnpEKzUwblRJbEFiSlJGOFpjSTBY?=
 =?utf-8?B?QkY3SVArckppamJlQTFIM1VCeEVXYXRZOGx4RG91ZUVTcHdkWkxFTDRzR2gy?=
 =?utf-8?B?a2huZDU2WFJPTkM1YllYRTVTaWp3SXM1Y0ZPYTkzVWx1NVJpM0tCMGNEYWFa?=
 =?utf-8?B?TlFpOFlJd2NtVEJRUGYyaThtTUtwRGxUL0RhVzlxWlduMTZWcU81Z2VJajNr?=
 =?utf-8?B?Q1NyU1RJSlRLU0pwL3pET0h4dzJtczJmQXk1bmZXbXVDbmFSMDBpNGFCcDdG?=
 =?utf-8?B?L2JvR3JCNmVkZldMM0Rtckx2T2JaWExEeHk2czVBdVJnaVVVWVQ0Nk82N3RV?=
 =?utf-8?B?dkV6YU5jOVJRVU9mVHBoVGJlbjdzRFlwdklFSHI1NmhQaUNPKzR6aWFzcDB2?=
 =?utf-8?B?L3pGR3R3V3dmaTFmVklRN1loa3R4aTF6Qm04aFhScVFpUjdMdGRIbjZMRUdl?=
 =?utf-8?B?eGdWdFVzaVdPOFV1OEdDTE9sbE9hSHdFYWtZcFNrWVZWTndQdjk2dUxGMEUw?=
 =?utf-8?B?cWJ6c0RLZlNDSmlVa1I5eVZzN2l2K0ZwQXRBaDN6eEdGUWtQMzhwcWFXWjhj?=
 =?utf-8?B?ZTBxN0psZGZCQUxtSGJHT0hrRml6ZENldlFFQUlZamhoYkw1OUN4TGhnMW1R?=
 =?utf-8?B?VzhRalAyTXByaTdyS3VhZzZDZ3RLQW1XbUJOb003V1ZwYm5xUlJvQlRveTdE?=
 =?utf-8?B?aTBXMFk2T3czOEpKbWdXRUloaWxGQURkaTBsWjc3WE9tSlp3UTVod25VaVZ1?=
 =?utf-8?B?dzd4Nkc0UnVoN0VCN2lKcmJKQ2hVdWlGc3oyeWZyTFJDY3oxUy9VZm9RRnhM?=
 =?utf-8?B?eTlxdWJsaXRETmtQa09aVzdMZWFYeUJDWTlOTDJsUzJHWDdGWmtRUklRQTlK?=
 =?utf-8?B?K1VUaXNhVzhsektEMElJd2Fvbm84aDY1WmlDK2s2QWJTMEs4VGRBVVEyL0x0?=
 =?utf-8?B?U0dESzR6T1BsUC9ZZExtRVN2QkNHOTA0Nnp4c3NQZXlDTWhISmlLbzVVUnNX?=
 =?utf-8?B?dzkzQ2tsS3hGTEJMMUZWSTVYOUEwMVlKYnVBWDJrelBZcUh5Q0lOQm5YNGZu?=
 =?utf-8?B?cUFRUzVaWXVLaEp2OVpoNVVYOXhVeW94VmwwUmRMdnhwbEZMTGRhVEVGRnVH?=
 =?utf-8?B?NmYvL0ZndmFzbmowUm1ybFZiRHcxNHJzcGxXMzRFY3dwYW5WY1lmaTN2SXlh?=
 =?utf-8?B?NFVuaUkzTlQvaEdxUGZvUTFMNU9uRGNkcjQrWEg2bXVKTWNWeDNNaStHbVRx?=
 =?utf-8?B?d0ZoME9tRjFYbEtNRU55UVU4NTlWUnY0YUFGWnE2MDZOOG5kSmdkZGpjMWR2?=
 =?utf-8?B?MjRNZG9ZN3lPc3oyWm1LTVlRRXNsa0xRQnAyaGplWDVwSGpQd1NsOVpwQlFk?=
 =?utf-8?B?TFBjR2x1UTcvNnVhMDdwaTZZUERjV1JYT0FpZldRcG5OeWpZcDZDaFhDZkVu?=
 =?utf-8?B?VGNNMVZGcWlQeWpFTGZXeEk5QTlXbGRTcFljQlEzWlpYSHpYMFFWc3VscWdn?=
 =?utf-8?B?azRSUkxHU0owTm1MM0Y1T2pVQUV4VU1oYUwvdlphRUhOY3o3RW1pZHl5QXh5?=
 =?utf-8?B?ekF4ZlZWRWxndyswWGVwQTVKS2xBS0s4aC92TjZYbU5RWjhXcVFNSGlXZE42?=
 =?utf-8?B?ckw3anQvaFY4aFBqY2RVdHI5YTJVUWk2NHRjU2Jlc1R2S3I4dkp2a2Frc3Mx?=
 =?utf-8?B?WitpYmVFbUtFU1JDaE1uUmxHbnVENG9kNnVzVWZnQTVmeGtRbkhVOWtEMDlM?=
 =?utf-8?B?YitTZlJBdXp1R0QvMDRkYzlyVDEvZER5eDRwcjRvaXZla2ZxeU04eGE4UEVm?=
 =?utf-8?B?K1RnODJ2clRMUzQ2a0hTMDVwaGF3cDdaKzIrUjcvdW51cHQ0Qng3RWhrL2xG?=
 =?utf-8?B?d2tRZ3hNRUROQ0gydWNMbFJIWlVVQWtwcWZxWnU0blg3aG5sbnZrUzNGc0Vn?=
 =?utf-8?B?TDFmczVtZTlWTWoyMDVWeE1CclNSTDN2cFIwbGF1ZTMwS2g1M0VJdDVNRnhD?=
 =?utf-8?B?dG5Sd2VBaHUvV0thVSszZS9Yc1JPcW9OR0pJTW5NcnhaL0YwaHRuckJXdnNl?=
 =?utf-8?Q?7S7GwBnZMtNWsuGX6CXtxaXrH?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0d05666d-3ee6-40fd-a9e1-08db3602deb0
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3370.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2023 18:23:35.8434
 (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: 6Y0CHZ1M+7gPmBEXTi1zqkCgrDVxwDompNMC6Leg9C+ECACFB9QOq8OzFCPTDhwv
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR12MB7428
X-Spam-Status: No, score=-0.6 required=5.0 tests=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
        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

On 2023-04-05 12:37, Asahi Lina wrote:
> After a job is pushed into the queue, it is owned by the scheduler core
> and may be freed at any time, so we can't write nor read the submit
> timestamp after that point.
> 
> Fixes oopses observed with the drm/asahi driver, found with kASAN.
> 
> Signed-off-by: Asahi Lina <lina@asahilina.net>
> ---

That's a good fix! Thanks for fixing this. I'll push
this through drm-misc-next and drm-misc-fixes, if there
are no objections.

Reviewed-by: Luben Tuikov <luben.tuikov@amd.com>

Regards,
Luben

>  drivers/gpu/drm/scheduler/sched_entity.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/scheduler/sched_entity.c b/drivers/gpu/drm/scheduler/sched_entity.c
> index 15d04a0ec623..e0a8890a62e2 100644
> --- a/drivers/gpu/drm/scheduler/sched_entity.c
> +++ b/drivers/gpu/drm/scheduler/sched_entity.c
> @@ -507,12 +507,19 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job)
>  {
>  	struct drm_sched_entity *entity = sched_job->entity;
>  	bool first;
> +	ktime_t submit_ts;
>  
>  	trace_drm_sched_job(sched_job, entity);
>  	atomic_inc(entity->rq->sched->score);
>  	WRITE_ONCE(entity->last_user, current->group_leader);
> +
> +	/*
> +	 * After the sched_job is pushed into the entity queue, it may be
> +	 * completed and freed up at any time. We can no longer access it.
> +	 * Make sure to set the submit_ts first, to avoid a race.
> +	 */
> +	sched_job->submit_ts = submit_ts = ktime_get();
>  	first = spsc_queue_push(&entity->job_queue, &sched_job->queue_node);
> -	sched_job->submit_ts = ktime_get();
>  
>  	/* first job wakes up scheduler */
>  	if (first) {
> @@ -529,7 +536,7 @@ void drm_sched_entity_push_job(struct drm_sched_job *sched_job)
>  		spin_unlock(&entity->rq_lock);
>  
>  		if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
> -			drm_sched_rq_update_fifo(entity, sched_job->submit_ts);
> +			drm_sched_rq_update_fifo(entity, submit_ts);
>  
>  		drm_sched_wakeup(entity->rq->sched);
>  	}
> 
> ---
> base-commit: fe15c26ee26efa11741a7b632e9f23b01aca4cc6
> change-id: 20230406-scheduler-uaf-2-44cf8faed245
> 
> Thank you,
> ~~ Lina
>