From 8f896b14d32cb338bb18489f3503eedd189c2dc4 Mon Sep 17 00:00:00 2001
From: Aidan MacDonald <amachronic@protonmail.com>
Date: Wed, 30 Mar 2022 20:55:14 +0100
Subject: [PATCH] buflib: remove the 'name' member from union buflib_data

Using a length 1 char array to point to the name buffer triggers
a -Warray-bounds warning from GCC when fortified strcpy is used.

This type of construct isn't safe in general -- if the compiler
makes assumptions based on the array bound it can create subtle
bugs when accessing the array out of bounds.

Instead, add a function get_block_name() which returns a pointer
to the name field by casting. This suppresses the warning and it
should be a bit more portable.

Change-Id: I25d4f46f799022ad0ec23bef0218f7595cc741ea
---
 firmware/buflib.c         | 13 +++++++++----
 firmware/include/buflib.h |  1 -
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/firmware/buflib.c b/firmware/buflib.c
index 6bfc499235..5cdb0f7ecd 100644
--- a/firmware/buflib.c
+++ b/firmware/buflib.c
@@ -187,6 +187,11 @@ static void check_block_crc(struct buflib_context *ctx,
                             union buflib_data *block,
                             union buflib_data *block_end);
 
+static inline char* get_block_name(union buflib_data *block)
+{
+    return (char*)&block[fidx_NAME];
+}
+
 /* Initialize buffer manager */
 void
 buflib_init(struct buflib_context *ctx, void *buf, size_t size)
@@ -378,7 +383,7 @@ move_block(struct buflib_context* ctx, union buflib_data* block, int shift)
 
     int handle = ctx->handle_table - h_entry;
     BDEBUGF("%s(): moving \"%s\"(id=%d) by %d(%d)\n", __func__,
-            block[fidx_NAME].name, handle, shift, shift*(int)sizeof(union buflib_data));
+            get_block_name(block), handle, shift, shift*(int)sizeof(union buflib_data));
     new_block = block + shift;
     new_start = h_entry->alloc + shift*sizeof(union buflib_data);
 
@@ -726,7 +731,7 @@ buffer_alloc:
     block[fidx_HANDLE].handle = handle;
     block[fidx_OPS].ops = ops;
     if (name_len > 0)
-        strcpy(block[fidx_NAME].name, name);
+        strcpy(get_block_name(block), name);
 
     size_t bsize = BUFLIB_NUM_FIELDS + name_len/sizeof(union buflib_data);
     union buflib_data *block_end = block + bsize;
@@ -1037,7 +1042,7 @@ const char* buflib_get_name(struct buflib_context *ctx, int handle)
         return NULL;
 
     data -= len;
-    return data[fidx_NAME].name;
+    return get_block_name(data);
 }
 
 #ifdef DEBUG
@@ -1095,7 +1100,7 @@ void buflib_print_block_at(struct buflib_context *ctx, int block_num,
         {
             snprintf(buf, bufsize, "%8p: val: %4ld (%s)",
                      block, (long)block->val,
-                     block->val > 0 ? block[fidx_NAME].name : "<unallocated>");
+                     block->val > 0 ? get_block_name(block) : "<unallocated>");
         }
     }
 }
diff --git a/firmware/include/buflib.h b/firmware/include/buflib.h
index a4796fbf33..45446c3b86 100644
--- a/firmware/include/buflib.h
+++ b/firmware/include/buflib.h
@@ -38,7 +38,6 @@ union buflib_data
     intptr_t val;                 /* length of the block in n*sizeof(union buflib_data).
                                      Includes buflib metadata overhead. A negative value
                                      indicates block is unallocated */
-    char name[1];                 /* name, actually a variable sized string */
     struct buflib_callbacks* ops; /* callback functions for move and shrink. Can be NULL */
     char* alloc;                  /* start of allocated memory area */
     union buflib_data *handle;    /* pointer to entry in the handle table.