summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAidan MacDonald <amachronic@protonmail.com>2022-03-30 20:55:14 +0100
committerAidan MacDonald <amachronic@protonmail.com>2022-09-19 15:09:51 -0400
commit8f896b14d32cb338bb18489f3503eedd189c2dc4 (patch)
tree6782d64b7660a06bcb87d0c7c13edad13ce709e4
parent6e9b1b344b7f7b04a2ae4ace0a1d191f63d03396 (diff)
downloadrockbox-8f896b14d32cb338bb18489f3503eedd189c2dc4.tar.gz
rockbox-8f896b14d32cb338bb18489f3503eedd189c2dc4.zip
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
-rw-r--r--firmware/buflib.c13
-rw-r--r--firmware/include/buflib.h1
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,
187 union buflib_data *block, 187 union buflib_data *block,
188 union buflib_data *block_end); 188 union buflib_data *block_end);
189 189
190static inline char* get_block_name(union buflib_data *block)
191{
192 return (char*)&block[fidx_NAME];
193}
194
190/* Initialize buffer manager */ 195/* Initialize buffer manager */
191void 196void
192buflib_init(struct buflib_context *ctx, void *buf, size_t size) 197buflib_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)
378 383
379 int handle = ctx->handle_table - h_entry; 384 int handle = ctx->handle_table - h_entry;
380 BDEBUGF("%s(): moving \"%s\"(id=%d) by %d(%d)\n", __func__, 385 BDEBUGF("%s(): moving \"%s\"(id=%d) by %d(%d)\n", __func__,
381 block[fidx_NAME].name, handle, shift, shift*(int)sizeof(union buflib_data)); 386 get_block_name(block), handle, shift, shift*(int)sizeof(union buflib_data));
382 new_block = block + shift; 387 new_block = block + shift;
383 new_start = h_entry->alloc + shift*sizeof(union buflib_data); 388 new_start = h_entry->alloc + shift*sizeof(union buflib_data);
384 389
@@ -726,7 +731,7 @@ buffer_alloc:
726 block[fidx_HANDLE].handle = handle; 731 block[fidx_HANDLE].handle = handle;
727 block[fidx_OPS].ops = ops; 732 block[fidx_OPS].ops = ops;
728 if (name_len > 0) 733 if (name_len > 0)
729 strcpy(block[fidx_NAME].name, name); 734 strcpy(get_block_name(block), name);
730 735
731 size_t bsize = BUFLIB_NUM_FIELDS + name_len/sizeof(union buflib_data); 736 size_t bsize = BUFLIB_NUM_FIELDS + name_len/sizeof(union buflib_data);
732 union buflib_data *block_end = block + bsize; 737 union buflib_data *block_end = block + bsize;
@@ -1037,7 +1042,7 @@ const char* buflib_get_name(struct buflib_context *ctx, int handle)
1037 return NULL; 1042 return NULL;
1038 1043
1039 data -= len; 1044 data -= len;
1040 return data[fidx_NAME].name; 1045 return get_block_name(data);
1041} 1046}
1042 1047
1043#ifdef DEBUG 1048#ifdef DEBUG
@@ -1095,7 +1100,7 @@ void buflib_print_block_at(struct buflib_context *ctx, int block_num,
1095 { 1100 {
1096 snprintf(buf, bufsize, "%8p: val: %4ld (%s)", 1101 snprintf(buf, bufsize, "%8p: val: %4ld (%s)",
1097 block, (long)block->val, 1102 block, (long)block->val,
1098 block->val > 0 ? block[fidx_NAME].name : "<unallocated>"); 1103 block->val > 0 ? get_block_name(block) : "<unallocated>");
1099 } 1104 }
1100 } 1105 }
1101} 1106}
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
38 intptr_t val; /* length of the block in n*sizeof(union buflib_data). 38 intptr_t val; /* length of the block in n*sizeof(union buflib_data).
39 Includes buflib metadata overhead. A negative value 39 Includes buflib metadata overhead. A negative value
40 indicates block is unallocated */ 40 indicates block is unallocated */
41 char name[1]; /* name, actually a variable sized string */
42 struct buflib_callbacks* ops; /* callback functions for move and shrink. Can be NULL */ 41 struct buflib_callbacks* ops; /* callback functions for move and shrink. Can be NULL */
43 char* alloc; /* start of allocated memory area */ 42 char* alloc; /* start of allocated memory area */
44 union buflib_data *handle; /* pointer to entry in the handle table. 43 union buflib_data *handle; /* pointer to entry in the handle table.