fw: Safer memory functions + asserts

Introduce memcpy_s() and wordcpy_s() that takes the destination buffer
size as an argument. Use assert() which aborts our program to an
eternal loop if we hit problems.

Sprinkle asserts elsewhere as well.

Signed-off-by: Daniel Lublin <daniel@lublin.se>

hw/application_fpga/Makefile

@@ -35,7 +35,8 @@ CC = clang
 
 CFLAGS = -target riscv32-unknown-none-elf -march=rv32iczmmul -mabi=ilp32 \
    -static -std=gnu99 -Os -ffast-math -fno-common -fno-builtin-printf \
-   -fno-builtin-putchar -nostdlib -mno-relax -Wall -flto -DNOCONSOLE
+   -fno-builtin-putchar -fno-builtin-memcpy -nostdlib -mno-relax -Wall \
+   -flto -DNOCONSOLE
 
 AS = clang
 ASFLAGS = -target riscv32-unknown-none-elf -march=rv32iczmmul -mabi=ilp32 -mno-relax
@@ -71,13 +72,15 @@ FIRMWARE_DEPS = \
 	$(P)/fw/tk1_mem.h \
 	$(P)/fw/tk1/types.h \
 	$(P)/fw/tk1/lib.h \
-	$(P)/fw/tk1/proto.h
+	$(P)/fw/tk1/proto.h \
+	$(P)/fw/tk1/assert.h \
 
 FIRMWARE_OBJS = \
 	$(P)/fw/tk1/main.o \
 	$(P)/fw/tk1/start.o \
 	$(P)/fw/tk1/proto.o \
 	$(P)/fw/tk1/lib.o \
+	$(P)/fw/tk1/assert.o \
 	$(P)/fw/tk1/blake2s/blake2s.o
 
 TESTFW_OBJS = \

hw/application_fpga/fw/tk1/Makefile

@@ -1,5 +1,5 @@
 # Uses ../.clang-format
-FMTFILES=main.c lib.h lib.c proto.h proto.c types.h
+FMTFILES=main.c lib.h lib.c proto.h proto.c types.h assert.c assert.h
 .PHONY: fmt
 fmt:
 	clang-format --dry-run --ferror-limit=0 $(FMTFILES)

hw/application_fpga/fw/tk1/assert.c

@@ -0,0 +1,25 @@
+/*
+ * Copyright (C) 2022, 2023 - Tillitis AB
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+
+#include "assert.h"
+#include "lib.h"
+
+void __assert_fail(const char *__assertion, const char *__file,
+		   unsigned int __line, const char *__function)
+{
+	htif_puts("assert: ");
+	htif_puts(__assertion);
+	htif_puts(" ");
+	htif_puts(__file);
+	htif_puts(":");
+	htif_putinthex(__line);
+	htif_puts(" ");
+	htif_puts(__function);
+	htif_lf();
+
+	for (;;);
+
+	// Not reached
+}

hw/application_fpga/fw/tk1/assert.h

@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) 2022, 2023 - Tillitis AB
+ * SPDX-License-Identifier: GPL-2.0-only
+ */
+
+#ifndef ASSERT_H
+#define ASSERT_H
+
+#define assert(expr)                                                           \
+	((expr) ? (void)(0)                                                    \
+		: __assert_fail(#expr, __FILE__, __LINE__, __func__))
+
+void __assert_fail(const char *__assertion, const char *__file,
+		   unsigned int __line, const char *__function);
+
+#endif

hw/application_fpga/fw/tk1/lib.c

@@ -5,6 +5,7 @@
 
 #include "lib.h"
 #include "types.h"
+#include "assert.h"
 
 #if NOCONSOLE
 void htif_putc(int ch)
@@ -131,28 +132,32 @@ void *memset(void *dest, int c, unsigned n)
 	return dest;
 }
 
-__attribute__((used)) void *memcpy(void *dest, const void *src, unsigned n)
+void memcpy_s(void *dest, size_t destsize, const void *src, size_t n)
 {
+	assert(dest != NULL);
+	assert(src != NULL);
+	assert(destsize >= n);
+
 	uint8_t *src_byte = (uint8_t *)src;
 	uint8_t *dest_byte = (uint8_t *)dest;
 
 	for (int i = 0; i < n; i++) {
 		dest_byte[i] = src_byte[i];
 	}
-
-	return dest;
 }
 
-__attribute__((used)) void *wordcpy(void *dest, const void *src, unsigned n)
+void wordcpy_s(void *dest, size_t destsize, const void *src, size_t n)
 {
+	assert(dest != NULL);
+	assert(src != NULL);
+	assert(destsize >= n);
+
 	uint32_t *src_word = (uint32_t *)src;
 	uint32_t *dest_word = (uint32_t *)dest;
 
 	for (int i = 0; i < n; i++) {
 		dest_word[i] = src_word[i];
 	}
-
-	return dest;
 }
 
 int memeq(void *dest, const void *src, unsigned n)

hw/application_fpga/fw/tk1/lib.h

@@ -15,8 +15,8 @@ void htif_putinthex(const uint32_t n);
 int htif_puts(const char *s);
 void htif_hexdump(uint8_t *buf, int len);
 void *memset(void *dest, int c, unsigned n);
-void *memcpy(void *dest, const void *src, unsigned n);
-void *wordcpy(void *dest, const void *src, unsigned n);
+void memcpy_s(void *dest, size_t destsize, const void *src, size_t n);
+void wordcpy_s(void *dest, size_t destsize, const void *src, size_t n);
 int memeq(void *dest, const void *src, unsigned n);
 
 #endif

hw/application_fpga/fw/tk1/main.c

@@ -8,6 +8,7 @@
 #include "lib.h"
 #include "proto.h"
 #include "types.h"
+#include "assert.h"
 
 // clang-format off
 static volatile uint32_t *uds             = (volatile uint32_t *)TK1_MMIO_UDS_FIRST;
@@ -118,10 +119,11 @@ static void compute_cdi(uint8_t digest[32], uint8_t use_uss, uint8_t uss[32])
 	}
 
 	// Only word aligned access to UDS
-	wordcpy((void *)fw_ram, (void *)uds, 8);
-	memcpy((void *)fw_ram + 32, digest, 32);
+	wordcpy_s((void *)fw_ram, TK1_MMIO_FW_RAM_SIZE, (void *)uds, 8);
+	memcpy_s((void *)&fw_ram[32], TK1_MMIO_FW_RAM_SIZE - 32, digest, 32);
 	if (use_uss != 0) {
-		memcpy((void *)fw_ram + 64, uss, 32);
+		memcpy_s((void *)&fw_ram[64], TK1_MMIO_FW_RAM_SIZE - 64, uss,
+			 32);
 		len = 96;
 	} else {
 		len = 64;
@@ -136,7 +138,7 @@ static void compute_cdi(uint8_t digest[32], uint8_t use_uss, uint8_t uss[32])
 	memset((void *)fw_ram, 0, TK1_MMIO_FW_RAM_SIZE);
 
 	// Only word aligned access to CDI
-	wordcpy((void *)cdi, (void *)local_cdi, 8);
+	wordcpy_s((void *)cdi, 8, (void *)local_cdi, 8);
 }
 
 void forever_redflash()
@@ -180,6 +182,9 @@ int main()
 			break;
 
 		case FW_STATE_INIT_LOADING:
+			assert(*app_size != 0);
+			assert(*app_size <= TK1_APP_MAX_SIZE);
+
 			*app_addr = 0;
 			left = *app_size;
 
@@ -269,9 +274,11 @@ int main()
 				break;
 			}
 
-			memcpy(rsp, &namever.name0, 4);
-			memcpy(rsp + 4, &namever.name1, 4);
-			memcpy(rsp + 8, &namever.version, 4);
+			memcpy_s(rsp, CMDLEN_MAXBYTES, &namever.name0, 4);
+			memcpy_s(&rsp[4], CMDLEN_MAXBYTES - 4, &namever.name1,
+				 4);
+			memcpy_s(&rsp[8], CMDLEN_MAXBYTES - 8, &namever.version,
+				 4);
 			fwreply(hdr, FW_RSP_NAME_VERSION, rsp);
 			// state unchanged
 			break;
@@ -287,8 +294,9 @@ int main()
 
 			rsp[0] = STATUS_OK;
 			uint32_t udi_words[2];
-			wordcpy(udi_words, (void *)udi, 2);
-			memcpy(rsp + 1, udi_words, 2 * 4);
+			wordcpy_s(udi_words, 2, (void *)udi, 2);
+			memcpy_s(&rsp[1], CMDLEN_MAXBYTES - 1, udi_words,
+				 2 * 4);
 			fwreply(hdr, FW_RSP_GET_UDI, rsp);
 			// state unchanged
 			break;
@@ -324,7 +332,7 @@ int main()
 			if (cmd[5] != 0) {
 				// Yes
 				use_uss = TRUE;
-				memcpy(uss, cmd + 6, 32);
+				memcpy_s(uss, 32, &cmd[6], 32);
 			} else {
 				use_uss = FALSE;
 			}
@@ -351,7 +359,7 @@ int main()
 			} else {
 				nbytes = left;
 			}
-			memcpy(loadaddr, cmd + 1, nbytes);
+			memcpy_s(loadaddr, left, cmd + 1, nbytes);
 			loadaddr += nbytes;
 			left -= nbytes;
 
@@ -371,7 +379,8 @@ int main()
 
 				// And return the digest in final response
 				rsp[0] = STATUS_OK;
-				memcpy(&rsp[1], &digest, 32);
+				memcpy_s(&rsp[1], CMDLEN_MAXBYTES - 1, &digest,
+					 32);
 				fwreply(hdr, FW_RSP_LOAD_APP_DATA_READY, rsp);
 
 				state = FW_STATE_RUN;