amdgpu: Make amdgpu_cs_signal_semaphore() thread-safe

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>

1 files changed, 11 insertions, 4 deletions

diff --git a/amdgpu/amdgpu_cs.c b/amdgpu/amdgpu_cs.c
index 49fc16c3..2db49675 100644
--- a/amdgpu/amdgpu_cs.c
+++ b/amdgpu/amdgpu_cs.c
@@ -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,