fdo-mirrors/mesa-drm
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/mesa/drm.git synced 2025-12-20 02:20:15 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

amdgpu: Make amdgpu_cs_signal_semaphore() thread-safe

Browse source 
The issue was found by a static analysis tool:

    Error: LOCK_EVASION (CWE-543):
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:596: thread1_checks_field:
        Thread1 uses the value read from field "context" in the
        condition "sem->signal_fence.context". It sees that the
        condition is false. Control is switched to Thread2.
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:596: thread2_checks_field:
        Thread2 uses the value read from field "context" in the
        condition "sem->signal_fence.context". It sees that the
        condition is false.
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:598: thread2_acquires_lock:
        Thread2 acquires lock "amdgpu_context.sequence_mutex".
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:599: thread2_modifies_field:
        Thread2 sets "context" to a new value. Note that this write can
        be reordered at runtime to occur before instructions that do
        not access this field within this locked region. After Thread2
        leaves the critical section, control is switched back to
        Thread1.
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:598: thread1_acquires_lock:
        Thread1 acquires lock "amdgpu_context.sequence_mutex".
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:599: thread1_overwrites_value_in_field:
        Thread1 sets "context" to a new value. Now the two threads have
        an inconsistent view of "context" and updates to fields of
        "context" or fields correlated with "context" may be lost.
    libdrm-2.4.115/amdgpu/amdgpu_cs.c:596: use_same_locks_for_read_and_modify:
        Guard the modification of "context" and the read used to decide
        whether to modify "context" with the same set of locks.
    #  597|                   return -EINVAL;
    #  598|           pthread_mutex_lock(&ctx->sequence_mutex);
    #  599|->         sem->signal_fence.context = ctx;
    #  600|           sem->signal_fence.ip_type = ip_type;
    #  601|           sem->signal_fence.ip_instance = ip_instance;

Check `sem->signal_fence.context` in the locked region to avoid a race
condition.

Reviewed-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>
Signed-off-by: José Expósito <jexposit@redhat.com>
This commit is contained in:
José Expósito 2024-03-21 11:41:18 +01:00 • committed by Pierre-Eric Pelloux-Prayer
parent 362b5b0a88
commit 4df9173595
1 changed files with 11 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
amdgpu/amdgpu_cs.c
View file

@ -598,24 +598,31 @@ drm_public int amdgpu_cs_signal_semaphore(amdgpu_context_handle ctx,
uint32_t ring,
amdgpu_semaphore_handle sem)
{
int ret;
if (!ctx || !sem)
return -EINVAL;
if (ip_type >= AMDGPU_HW_IP_NUM)
return -EINVAL;
if (ring >= AMDGPU_CS_MAX_RINGS)
return -EINVAL;
/* sem has been signaled */
if (sem->signal_fence.context)
return -EINVAL;
pthread_mutex_lock(&ctx->sequence_mutex);
/* sem has been signaled */
if (sem->signal_fence.context) {
ret = -EINVAL;
goto unlock;
}
sem->signal_fence.context = ctx;
sem->signal_fence.ip_type = ip_type;
sem->signal_fence.ip_instance = ip_instance;
sem->signal_fence.ring = ring;
sem->signal_fence.fence = ctx->last_seq[ip_type][ip_instance][ring];
update_references(NULL, &sem->refcount);
ret = 0;
unlock:
pthread_mutex_unlock(&ctx->sequence_mutex);
return 0;
return ret;
}
drm_public int amdgpu_cs_wait_semaphore(amdgpu_context_handle ctx,