intel/ds: Fix crash when allocating more intel_ds_queues than u_vector was initialized

u_vector_add() don't keep the returned pointers valid.
After the initial size allocated in u_vector_init() is reached it will
allocate a bigger buffer and copy data from older buffer to the new
one and free the old buffer, making all the previous pointers returned
by u_vector_add() invalid and crashing the application when trying to
access it.

This is reproduced when running
dEQP-VK.synchronization.signal_order.timeline_semaphore.* in DG2 SKUs
that has 4 CCS engines, INTEL_COMPUTE_CLASS=1 is set and of course
perfetto build is enabled.

To fix this issue here I'm moving the storage/allocation of
struct intel_ds_queue to struct anv_queue/iris_batch and using
struct list_head to maintain a chain of intel_ds_queue of the
intel_ds_device.
This allows us to append or remove queues dynamically in future if
necessary.

Fixes: e760c5b37be9 ("anv: add perfetto source")
Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20977>
(cherry picked from commit 8092bc2158ebb8a5f85e0ec569387c5dcd0d1627)

diff --git a/.pick_status.json b/.pick_status.json
index 42ac2af..52f1a4e 100644 (file)
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -3568,7 +3568,7 @@
         "description": "intel/ds: Fix crash when allocating more intel_ds_queues than u_vector was initialized",
         "nominated": true,
         "nomination_type": 1,
-        "resolution": 0,
+        "resolution": 1,
         "main_sha": null,
         "because_sha": "e760c5b37be938427a9c88182ea99f7f66721ca3"
     },

diff --git a/src/gallium/drivers/iris/iris_batch.c b/src/gallium/drivers/iris/iris_batch.c
index ec32d88..d598fd7 100644 (file)
--- a/src/gallium/drivers/iris/iris_batch.c
+++ b/src/gallium/drivers/iris/iris_batch.c
@@ -1053,10 +1053,10 @@ _iris_batch_flush(struct iris_batch *batch, const char *file, int line)
 
    }
 
-   uint64_t start_ts = intel_ds_begin_submit(batch->ds);
-   uint64_t submission_id = batch->ds->submission_id;
+   uint64_t start_ts = intel_ds_begin_submit(&batch->ds);
+   uint64_t submission_id = batch->ds.submission_id;
    int ret = submit_batch(batch);
-   intel_ds_end_submit(batch->ds, start_ts);
+   intel_ds_end_submit(&batch->ds, start_ts);
 
    /* When batch submission fails, our end-of-batch syncobj remains
     * unsignalled, and in fact is not even considered submitted.

diff --git a/src/gallium/drivers/iris/iris_batch.h b/src/gallium/drivers/iris/iris_batch.h
index a1dfa6e..437352a 100644 (file)
--- a/src/gallium/drivers/iris/iris_batch.h
+++ b/src/gallium/drivers/iris/iris_batch.h
@@ -197,7 +197,7 @@ struct iris_batch {
    struct u_trace trace;
 
    /** Batch wrapper structure for perfetto */
-   struct intel_ds_queue *ds;
+   struct intel_ds_queue ds;
 };
 
 void iris_init_batches(struct iris_context *ice, int priority);

diff --git a/src/gallium/drivers/iris/iris_utrace.c b/src/gallium/drivers/iris/iris_utrace.c
index 7f49826..e66a560 100644 (file)
--- a/src/gallium/drivers/iris/iris_utrace.c
+++ b/src/gallium/drivers/iris/iris_utrace.c
@@ -95,7 +95,7 @@ iris_utrace_delete_flush_data(struct u_trace_context *utctx,
 void iris_utrace_flush(struct iris_batch *batch, uint64_t submission_id)
 {
    struct intel_ds_flush_data *flush_data = malloc(sizeof(*flush_data));
-   intel_ds_flush_data_init(flush_data, batch->ds, submission_id);
+   intel_ds_flush_data_init(flush_data, &batch->ds, submission_id);
    u_trace_flush(&batch->trace, flush_data, false);
 }
 
@@ -122,9 +122,8 @@ void iris_utrace_init(struct iris_context *ice)
                              iris_utrace_delete_flush_data);
 
    for (int i = 0; i < IRIS_BATCH_COUNT; i++) {
-      ice->batches[i].ds =
-         intel_ds_device_add_queue(&ice->ds, "%s",
-                                   iris_batch_name_to_string(i));
+      intel_ds_device_init_queue(&ice->ds, &ice->batches[i].ds, "%s",
+                                 iris_batch_name_to_string(i));
    }
 }
 

diff --git a/src/intel/ds/intel_driver_ds.cc b/src/intel/ds/intel_driver_ds.cc
index 516be6d..dc51faa 100644 (file)
--- a/src/intel/ds/intel_driver_ds.cc
+++ b/src/intel/ds/intel_driver_ds.cc
@@ -185,12 +185,10 @@ static void
 send_descriptors(IntelRenderpassDataSource::TraceContext &ctx,
                  struct intel_ds_device *device)
 {
-   struct intel_ds_queue *queue;
-
    PERFETTO_LOG("Sending renderstage descriptors");
 
    device->event_id = 0;
-   u_vector_foreach(queue, &device->queues) {
+   list_for_each_entry_safe(struct intel_ds_queue, queue, &device->queues, link) {
       for (uint32_t s = 0; s < ARRAY_SIZE(queue->stages); s++) {
          queue->stages[s].start_ns = 0;
       }
@@ -222,7 +220,7 @@ send_descriptors(IntelRenderpassDataSource::TraceContext &ctx,
       }
 
       /* Emit all the IID picked at device/queue creation. */
-      u_vector_foreach(queue, &device->queues) {
+      list_for_each_entry_safe(struct intel_ds_queue, queue, &device->queues, link) {
          for (unsigned s = 0; s < INTEL_DS_QUEUE_STAGE_N_STAGES; s++) {
             {
                /* We put the stage number in there so that all rows are order
@@ -528,23 +526,21 @@ intel_ds_device_init(struct intel_ds_device *device,
    device->info = *devinfo;
    device->iid = get_iid();
    device->api = api;
-   u_vector_init(&device->queues, 4, sizeof(struct intel_ds_queue));
+   list_inithead(&device->queues);
 }
 
 void
 intel_ds_device_fini(struct intel_ds_device *device)
 {
    u_trace_context_fini(&device->trace_context);
-   u_vector_finish(&device->queues);
 }
 
 struct intel_ds_queue *
-intel_ds_device_add_queue(struct intel_ds_device *device,
-                          const char *fmt_name,
-                          ...)
+intel_ds_device_init_queue(struct intel_ds_device *device,
+                           struct intel_ds_queue *queue,
+                           const char *fmt_name,
+                           ...)
 {
-   struct intel_ds_queue *queue =
-      (struct intel_ds_queue *) u_vector_add(&device->queues);
    va_list ap;
 
    memset(queue, 0, sizeof(*queue));
@@ -560,6 +556,8 @@ intel_ds_device_add_queue(struct intel_ds_device *device,
       queue->stages[s].stage_iid = get_iid();
    }
 
+   list_add(&queue->link, &device->queues);
+
    return queue;
 }
 

diff --git a/src/intel/ds/intel_driver_ds.h b/src/intel/ds/intel_driver_ds.h
index b5e5091..f88f5f7 100644 (file)
--- a/src/intel/ds/intel_driver_ds.h
+++ b/src/intel/ds/intel_driver_ds.h
@@ -107,7 +107,7 @@ struct intel_ds_device {
    struct u_trace_context trace_context;
 
    /* List of intel_ds_queue */
-   struct u_vector queues;
+   struct list_head queues;
 };
 
 struct intel_ds_stage {
@@ -122,6 +122,8 @@ struct intel_ds_stage {
 };
 
 struct intel_ds_queue {
+   struct list_head link;
+
    /* Device this queue belongs to */
    struct intel_ds_device *device;
 
@@ -155,9 +157,11 @@ void intel_ds_device_init(struct intel_ds_device *device,
                           enum intel_ds_api api);
 void intel_ds_device_fini(struct intel_ds_device *device);
 
-struct intel_ds_queue *intel_ds_device_add_queue(struct intel_ds_device *device,
-                                                 const char *fmt_name,
-                                                 ...);
+struct intel_ds_queue *
+intel_ds_device_init_queue(struct intel_ds_device *device,
+                           struct intel_ds_queue *queue,
+                           const char *fmt_name,
+                           ...);
 
 void intel_ds_flush_data_init(struct intel_ds_flush_data *data,
                               struct intel_ds_queue *queue,

diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c
index 3fd06fd..cf890bd 100644 (file)
--- a/src/intel/vulkan/anv_batch_chain.c
+++ b/src/intel/vulkan/anv_batch_chain.c
@@ -1991,14 +1991,14 @@ anv_queue_submit(struct vk_queue *vk_queue,
       return VK_SUCCESS;
    }
 
-   uint64_t start_ts = intel_ds_begin_submit(queue->ds);
+   uint64_t start_ts = intel_ds_begin_submit(&queue->ds);
 
    pthread_mutex_lock(&device->mutex);
    result = anv_queue_submit_locked(queue, submit);
    /* Take submission ID under lock */
    pthread_mutex_unlock(&device->mutex);
 
-   intel_ds_end_submit(queue->ds, start_ts);
+   intel_ds_end_submit(&queue->ds, start_ts);
 
    return result;
 }

diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h
index 01e1a46..0b2e428 100644 (file)
--- a/src/intel/vulkan/anv_private.h
+++ b/src/intel/vulkan/anv_private.h
@@ -1102,7 +1102,7 @@ struct anv_queue {
    /** Synchronization object for debug purposes (DEBUG_SYNC) */
    struct vk_sync                           *sync;
 
-   struct intel_ds_queue *                   ds;
+   struct intel_ds_queue                     ds;
 };
 
 struct nir_xfb_info;

diff --git a/src/intel/vulkan/anv_utrace.c b/src/intel/vulkan/anv_utrace.c
index 35a744d..3a35aef 100644 (file)
--- a/src/intel/vulkan/anv_utrace.c
+++ b/src/intel/vulkan/anv_utrace.c
@@ -111,7 +111,7 @@ anv_device_utrace_flush_cmd_buffers(struct anv_queue *queue,
    if (!flush)
       return vk_error(device, VK_ERROR_OUT_OF_HOST_MEMORY);
 
-   intel_ds_flush_data_init(&flush->ds, queue->ds, queue->ds->submission_id);
+   intel_ds_flush_data_init(&flush->ds, &queue->ds, queue->ds.submission_id);
 
    result = vk_sync_create(&device->vk, &device->physical->sync_syncobj_type,
                            0, 0, &flush->sync);
@@ -284,8 +284,7 @@ anv_device_utrace_init(struct anv_device *device)
    for (uint32_t q = 0; q < device->queue_count; q++) {
       struct anv_queue *queue = &device->queues[q];
 
-      queue->ds =
-         intel_ds_device_add_queue(&device->ds, "%s%u",
+      intel_ds_device_init_queue(&device->ds, &queue->ds, "%s%u",
                                    intel_engines_class_to_string(queue->family->engine_class),
                                    queue->index_in_family);
    }

diff --git a/src/intel/vulkan_hasvk/anv_batch_chain.c b/src/intel/vulkan_hasvk/anv_batch_chain.c
index 1a14a34..459f0df 100644 (file)
--- a/src/intel/vulkan_hasvk/anv_batch_chain.c
+++ b/src/intel/vulkan_hasvk/anv_batch_chain.c
@@ -2393,14 +2393,14 @@ anv_queue_submit(struct vk_queue *vk_queue,
       return VK_SUCCESS;
    }
 
-   uint64_t start_ts = intel_ds_begin_submit(queue->ds);
+   uint64_t start_ts = intel_ds_begin_submit(&queue->ds);
 
    pthread_mutex_lock(&device->mutex);
    result = anv_queue_submit_locked(queue, submit);
    /* Take submission ID under lock */
    pthread_mutex_unlock(&device->mutex);
 
-   intel_ds_end_submit(queue->ds, start_ts);
+   intel_ds_end_submit(&queue->ds, start_ts);
 
    return result;
 }

diff --git a/src/intel/vulkan_hasvk/anv_private.h b/src/intel/vulkan_hasvk/anv_private.h
index 0367cef..6116afb 100644 (file)
--- a/src/intel/vulkan_hasvk/anv_private.h
+++ b/src/intel/vulkan_hasvk/anv_private.h
@@ -1074,7 +1074,7 @@ struct anv_queue {
    /** Synchronization object for debug purposes (DEBUG_SYNC) */
    struct vk_sync                           *sync;
 
-   struct intel_ds_queue *                   ds;
+   struct intel_ds_queue                     ds;
 };
 
 struct nir_xfb_info;

diff --git a/src/intel/vulkan_hasvk/anv_utrace.c b/src/intel/vulkan_hasvk/anv_utrace.c
index 35a744d..3a35aef 100644 (file)
--- a/src/intel/vulkan_hasvk/anv_utrace.c
+++ b/src/intel/vulkan_hasvk/anv_utrace.c
@@ -111,7 +111,7 @@ anv_device_utrace_flush_cmd_buffers(struct anv_queue *queue,
    if (!flush)
       return vk_error(device, VK_ERROR_OUT_OF_HOST_MEMORY);
 
-   intel_ds_flush_data_init(&flush->ds, queue->ds, queue->ds->submission_id);
+   intel_ds_flush_data_init(&flush->ds, &queue->ds, queue->ds.submission_id);
 
    result = vk_sync_create(&device->vk, &device->physical->sync_syncobj_type,
                            0, 0, &flush->sync);
@@ -284,8 +284,7 @@ anv_device_utrace_init(struct anv_device *device)
    for (uint32_t q = 0; q < device->queue_count; q++) {
       struct anv_queue *queue = &device->queues[q];
 
-      queue->ds =
-         intel_ds_device_add_queue(&device->ds, "%s%u",
+      intel_ds_device_init_queue(&device->ds, &queue->ds, "%s%u",
                                    intel_engines_class_to_string(queue->family->engine_class),
                                    queue->index_in_family);
    }