alloc: fix potential oob in rrealloc

Previous memcpy of bytes is unsafe if new object is larger then old one (which is probably most popular realloc use). Typically realloc retrieves the old size internally and does not need this information passed as argument but sof heap implementation does not store the exact size of the allocated memory buffer. Signed-off-by: Marcin Maka <marcin.maka@linux.intel.com>
iganakov · Apr 8, 2020 · 75b8c1e · 75b8c1e
1 parent 5e3bc9c
commit 75b8c1e
Show file tree

Hide file tree

Showing 5 changed files with 10 additions and 6 deletions.
diff --git a/src/audio/component.c b/src/audio/component.c
@@ -325,7 +325,7 @@ struct comp_dev *comp_make_shared(struct comp_dev *dev)
 	struct comp_dev *old = dev;
 
 	dev = rrealloc(dev, SOF_MEM_ZONE_RUNTIME, SOF_MEM_FLAG_SHARED,
-		       SOF_MEM_CAPS_RAM, dev->size);
+		       SOF_MEM_CAPS_RAM, dev->size, dev->size);
 	if (!dev) {
 		trace_error(TRACE_CLASS_COMP, "comp_make_shared(): unable to realloc component");
 		return NULL;

diff --git a/src/include/sof/lib/alloc.h b/src/include/sof/lib/alloc.h
@@ -111,13 +111,14 @@ static inline void *rballoc(uint32_t flags, uint32_t caps, size_t bytes)
  * @param flags Flags, see SOF_MEM_FLAG_...
  * @param caps Capabilities, see SOF_MEM_CAPS_...
  * @param bytes New size in bytes.
+ * @param old_bytes Old size in bytes.
  * @return Pointer to the resized memory or NULL if failed.
  *
  * Note: Do not use for buffers (SOF_MEM_ZONE_BUFFER zone).
  * Use rballoc(), rballoc_align() to allocate memory for buffers.
  */
 void *rrealloc(void *ptr, enum mem_zone zone, uint32_t flags, uint32_t caps,
-	       size_t bytes);
+	       size_t bytes, size_t old_bytes);
 
 /**
  * Changes size of the memory block allocated from SOF_MEM_ZONE_BUFFER.

diff --git a/src/lib/alloc.c b/src/lib/alloc.c
@@ -12,6 +12,7 @@
 #include <sof/lib/dma.h>
 #include <sof/lib/memory.h>
 #include <sof/lib/mm_heap.h>
+#include <sof/math/numbers.h>
 #include <sof/spinlock.h>
 #include <sof/string.h>
 #include <ipc/topology.h>
@@ -913,11 +914,12 @@ void rfree(void *ptr)
 }
 
 void *rrealloc(void *ptr, enum mem_zone zone, uint32_t flags, uint32_t caps,
-	       size_t bytes)
+	       size_t bytes, size_t old_bytes)
 {
 	struct mm *memmap = memmap_get();
 	void *new_ptr = NULL;
 	uint32_t lock_flags;
+	size_t copy_bytes = MIN(bytes, old_bytes);
 
 	if (!bytes)
 		return new_ptr;
@@ -927,7 +929,7 @@ void *rrealloc(void *ptr, enum mem_zone zone, uint32_t flags, uint32_t caps,
 	new_ptr = _malloc_unlocked(zone, flags, caps, bytes);
 
 	if (new_ptr && ptr)
-		memcpy_s(new_ptr, bytes, ptr, bytes);
+		memcpy_s(new_ptr, copy_bytes, ptr, copy_bytes);
 
 	if (new_ptr)
 		_rfree_unlocked(ptr);

diff --git a/test/cmocka/src/audio/component/mock.c b/test/cmocka/src/audio/component/mock.c
@@ -29,12 +29,13 @@ void *rzalloc(enum mem_zone zone, uint32_t flags, uint32_t caps, size_t bytes)
 }
 
 void *rrealloc(void *ptr, enum mem_zone zone, uint32_t flags, uint32_t caps,
-	       size_t bytes)
+	       size_t bytes, size_t old_bytes)
 {
 	(void)ptr;
 	(void)zone;
 	(void)flags;
 	(void)caps;
+	(void)old_bytes;
 
 	return realloc(ptr, bytes);
 }

diff --git a/tools/testbench/alloc.c b/tools/testbench/alloc.c
@@ -39,7 +39,7 @@ void *rballoc_align(uint32_t flags, uint32_t caps, size_t bytes,
 }
 
 void *rrealloc(void *ptr, enum mem_zone zone, uint32_t flags, uint32_t caps,
-	       size_t bytes)
+	       size_t bytes, size_t old_bytes)
 {
 	return realloc(ptr, bytes);
 }