In aca_error_cache_log_bank_error(), find_bank_error() released
aerr->lock prior to returning bank_error. This created a time-of-check
to time-of-use (TOCTOU) race window where a concurrent caller of
aca_log_aca_error() could acquire aerr->lock and free the bank_error
node via aca_bank_error_remove().

When execution returned to aca_error_cache_log_bank_error(),
incrementing bank_error->count resulted in a Use-After-Free and
potential kernel memory corruption. Additionally, bank_error->count
was updated outside mutex lock protection.

Fix this by acquiring aerr->lock at the start of
aca_error_cache_log_bank_error() and holding it continuously across
lookup, creation, and counter updates, while removing redundant
internal lock acquisitions in helper functions.

Signed-off-by: Sreeraj S Kurup <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c | 18 +++++++-----------
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
index db7858fe0c3d..d0d473082431 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_aca.c
@@ -237,10 +237,8 @@ static struct aca_bank_error *new_bank_error(struct 
aca_error *aerr, struct aca_
        INIT_LIST_HEAD(&bank_error->node);
        memcpy(&bank_error->info, info, sizeof(*info));
 
-       mutex_lock(&aerr->lock);
        list_add_tail(&bank_error->node, &aerr->list);
        aerr->nr_errors++;
-       mutex_unlock(&aerr->lock);
 
        return bank_error;
 }
@@ -249,22 +247,16 @@ static struct aca_bank_error *find_bank_error(struct 
aca_error *aerr, struct aca
 {
        struct aca_bank_error *bank_error = NULL;
        struct aca_bank_info *tmp_info;
-       bool found = false;
 
-       mutex_lock(&aerr->lock);
        list_for_each_entry(bank_error, &aerr->list, node) {
                tmp_info = &bank_error->info;
                if (tmp_info->socket_id == info->socket_id &&
                    tmp_info->die_id == info->die_id) {
-                       found = true;
-                       goto out_unlock;
+                       return bank_error;
                }
        }
 
-out_unlock:
-       mutex_unlock(&aerr->lock);
-
-       return found ? bank_error : NULL;
+       return NULL;
 }
 
 static void aca_bank_error_remove(struct aca_error *aerr, struct 
aca_bank_error *bank_error)
@@ -306,11 +298,15 @@ int aca_error_cache_log_bank_error(struct aca_handle 
*handle, struct aca_bank_in
                return 0;
 
        aerr = &error_cache->errors[type];
+       mutex_lock(&aerr->lock);
        bank_error = get_bank_error(aerr, info);
-       if (!bank_error)
+       if (!bank_error) {
+               mutex_unlock(&aerr->lock);
                return -ENOMEM;
+       }
 
        bank_error->count += count;
+       mutex_unlock(&aerr->lock);
 
        return 0;
 }
-- 
2.54.0

Reply via email to