Re: [PATCH 1/1] tee: amdtee: fix race condition in amdtee_open_session

On Tue, 28 Feb 2023 at 15:11, Rijo Thomas Rijo-john.Thomas@amd.com wrote:

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().

Fixes: 757cc3e9ff1d ("tee: add AMD-TEE driver") Cc: stable@vger.kernel.org Signed-off-by: Rijo Thomas Rijo-john.Thomas@amd.com

---

drivers/tee/amdtee/core.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-)

Looks sane to me, FWIW:

Acked-by: Sumit Garg sumit.garg@linaro.org

-Sumit

diff --git a/drivers/tee/amdtee/core.c b/drivers/tee/amdtee/core.c index 297dc62bca29..372d64756ed6 100644 --- a/drivers/tee/amdtee/core.c +++ b/drivers/tee/amdtee/core.c @@ -267,35 +267,34 @@ int amdtee_open_session(struct tee_context *ctx, 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;
  

•   }
  

•   /* Find an empty session index for the given TA */
    spin_lock(&sess->lock);
    i = find_first_zero_bit(sess->sess_mask, TEE_NUM_SESSIONS);
  

•   if (i < TEE_NUM_SESSIONS)
  

•   if (i < TEE_NUM_SESSIONS) {
  

•           sess->session_info[i] = session_info;
  

•           set_session_id(ta_handle, i, &arg->session);
            set_bit(i, sess->sess_mask);
  

•   }
    spin_unlock(&sess->lock);
  
    if (i >= TEE_NUM_SESSIONS) {
            pr_err("reached maximum session count %d\n", TEE_NUM_SESSIONS);
  

•           handle_close_session(ta_handle, session_info);
            handle_unload_ta(ta_handle);
            kref_put(&sess->refcount, destroy_session);
            rc = -ENOMEM;
            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);
  

•           spin_lock(&sess->lock);
  

•           clear_bit(i, sess->sess_mask);
  

•           spin_unlock(&sess->lock);
  

•           handle_unload_ta(ta_handle);
  

•           kref_put(&sess->refcount, destroy_session);
  

•           goto out;
  

•   }
  

•   sess->session_info[i] = session_info;
  

•   set_session_id(ta_handle, i, &arg->session);
  

out: free_pages((u64)ta, get_order(ta_size)); return rc; -- 2.25.1