On 4/4/2023 5:01 PM, Pavel Machek wrote:
> Hi!
>
>> commit f8502fba45bd30e1a6a354d9d898bc99d1a11e6d upstream.
>>
>> There is a potential race condition in amdtee_open_session that may
>> lead to use-after-free. For instance, in amdtee_open_session() after
>> sess->sess_mask is set, and before setting:
>>
>> sess->session_info[i] = session_info;
>>
>> if amdtee_close_session() closes this same session, then 'sess' data
>> structure will be released, causing kernel panic when 'sess' is
>> accessed within amdtee_open_session().
>>
>> The solution is to set the bit sess->sess_mask as the last step in
>> amdtee_open_session().
>
> Ok, but:
>
>> +++ b/drivers/tee/amdtee/core.c
>> @@ -267,35 +267,34 @@ int amdtee_open_session(struct tee_conte
>> goto out;
>> }
>>
>> + /* Open session with loaded TA */
>> + handle_open_session(arg, &session_info, param);
>> + if (arg->ret != TEEC_SUCCESS) {
>> + pr_err("open_session failed %d\n", arg->ret);
>> + handle_unload_ta(ta_handle);
>> + kref_put(&sess->refcount, destroy_session);
>> + goto out;
>> + }
>
> rc needs to be set to something here, otherwise we'll return 0 below.
>
Sorry about the delay in my response. Somehow missed this email.
Yes, that is the expected behavior. rc will be 0 while arg->ret will have
the return status.
Thanks,
Rijo
>> out:
>> free_pages((u64)ta, get_order(ta_size));
>> return rc;
>
> Best regards,
> Pavel
>
