buffer: prevent cache corruption

Recent research discovered a set of potential issues related with
cache prefetch. Specifically it seems like uncached access to memory
can cause cache prefetch. This can cause problems in buffer_attach()
and buffer_detach() where buffers are added to or removed from lists
respectively via uncached addresses, after which they can be used via
cached addresses. Add proper cache synchronisation and interrupt
locking to protect against such memory corruption.

Signed-off-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>

src/audio/buffer.c

@@ -286,8 +286,15 @@ void comp_update_buffer_consume(struct comp_buffer __sparse_cache *buffer, uint3
 		 (char *)audio_stream_get_addr(&buffer->stream)));
 }
 
+/*
+ * Locking: must be called with interrupts disabled! Serialized IPCs protect us
+ * from racing attach / detach calls, but the scheduler can interrupt the IPC
+ * thread and begin using the buffer for streaming. FIXME: this is still a
+ * problem with different cores.
+ */
 void buffer_attach(struct comp_buffer *buffer, struct list_item *head, int dir)
 {
+	struct list_item *list = buffer_comp_list(buffer, dir);
 	struct list_item __sparse_cache *needs_sync;
 	bool further_buffers_exist;
 
@@ -302,11 +309,17 @@ void buffer_attach(struct comp_buffer *buffer, struct list_item *head, int dir)
 	if (further_buffers_exist)
 		dcache_writeback_region(needs_sync, sizeof(struct list_item));
 	/* The cache line can be prefetched here, invalidate it after prepending */
-	list_item_prepend(buffer_comp_list(buffer, dir), head);
+	list_item_prepend(list, head);
 	if (further_buffers_exist)
 		dcache_invalidate_region(needs_sync, sizeof(struct list_item));
+	/* no dirty cache lines exist for this buffer yet, no need to write back */
+	dcache_invalidate_region(uncache_to_cache(list), sizeof(*list));
 }
 
+/*
+ * Locking: must be called with interrupts disabled! See buffer_attach() above
+ * for details
+ */
 void buffer_detach(struct comp_buffer *buffer, struct list_item *head, int dir)
 {
 	struct list_item __sparse_cache *needs_sync_prev, *needs_sync_next;
@@ -329,8 +342,10 @@ void buffer_detach(struct comp_buffer *buffer, struct list_item *head, int dir)
 		dcache_writeback_region(needs_sync_next, sizeof(struct list_item));
 	if (buffers_before_exist)
 		dcache_writeback_region(needs_sync_prev, sizeof(struct list_item));
+	dcache_writeback_region(uncache_to_cache(buf_list), sizeof(*buf_list));
 	/* buffers before or after can be prefetched here */
 	list_item_del(buf_list);
+	dcache_invalidate_region(uncache_to_cache(buf_list), sizeof(*buf_list));
 	if (buffers_after_exist)
 		dcache_invalidate_region(needs_sync_next, sizeof(struct list_item));
 	if (buffers_before_exist)

src/audio/module_adapter/module_adapter.c

@@ -18,6 +18,7 @@
 #include <sof/common.h>
 #include <sof/platform.h>
 #include <sof/ut.h>
+#include <rtos/interrupt.h>
 #include <limits.h>
 #include <stdint.h>
 
@@ -439,13 +440,17 @@ int module_adapter_prepare(struct comp_dev *dev)
 		for (i = 0; i < mod->num_output_buffers; i++) {
 			struct comp_buffer *buffer = buffer_alloc(buff_size, SOF_MEM_CAPS_RAM,
 								  0, PLATFORM_DCACHE_ALIGN);
+			uint32_t flags;
+
 			if (!buffer) {
 				comp_err(dev, "module_adapter_prepare(): failed to allocate local buffer");
 				ret = -ENOMEM;
 				goto free;
 			}
 
+			irq_local_disable(flags);
 			buffer_attach(buffer, &mod->sink_buffer_list, PPL_DIR_UPSTREAM);
+			irq_local_enable(flags);
 
 			buffer_c = buffer_acquire(buffer);
 			buffer_set_params(buffer_c, mod->stream_params, BUFFER_UPDATE_FORCE);
@@ -480,8 +485,11 @@ int module_adapter_prepare(struct comp_dev *dev)
 	list_for_item_safe(blist, _blist, &mod->sink_buffer_list) {
 		struct comp_buffer *buffer = container_of(blist, struct comp_buffer,
 							  sink_list);
+		uint32_t flags;
 
+		irq_local_disable(flags);
 		buffer_detach(buffer, &mod->sink_buffer_list, PPL_DIR_UPSTREAM);
+		irq_local_enable(flags);
 		buffer_free(buffer);
 	}
 
@@ -1404,8 +1412,11 @@ void module_adapter_free(struct comp_dev *dev)
 	list_for_item_safe(blist, _blist, &mod->sink_buffer_list) {
 		struct comp_buffer *buffer = container_of(blist, struct comp_buffer,
 							  sink_list);
+		uint32_t flags;
 
+		irq_local_disable(flags);
 		buffer_detach(buffer, &mod->sink_buffer_list, PPL_DIR_UPSTREAM);
+		irq_local_enable(flags);
 		buffer_free(buffer);
 	}