llext: fix fixed-length name buffer usage

This was inspired by the detection of 2 instances of the warning:

   warning: 'strncpy' specified bound 16 equals destination size
            [-Wstringop-truncation]

The current code is already safe with regards to overflows, because
fixed-length string functions are used in the call tree. However, when
given a name 16 chars or larger, the current compare in llext_by_name()
will not work as expected because the stored extension name is truncated
to a max of 15.

Define a global LLEXT_MAX_NAME_LEN constant to simplify all this logic
and also implement name checks in the shell before calling llext_load().

Finally, using strlen() instead of strnlen() gets the real length of the
hex string passed as a parameter, which is important for the next safety
check.

Signed-off-by: Luca Burelli <[email protected]>

Author: pillo79

include/zephyr/llext/llext.h

@@ -65,7 +65,10 @@ enum llext_mem {
 struct llext_loader;
 /** @endcond */
 
-/* Maximim number of dependency LLEXTs */
+/** Maximum length of an extension name */
+#define LLEXT_MAX_NAME_LEN 15
+
+/** Maximum number of dependency LLEXTs */
 #define LLEXT_MAX_DEPENDENCIES 8
 
 /**
@@ -86,7 +89,7 @@ struct llext {
 	/** @endcond */
 
 	/** Name of the llext */
-	char name[16];
+	char name[LLEXT_MAX_NAME_LEN + 1];
 
 	/** Lookup table of memory regions */
 	void *mem[LLEXT_MEM_COUNT];

subsys/llext/llext.c

@@ -95,7 +95,7 @@ struct llext *llext_by_name(const char *name)
 	     node = sys_slist_peek_next(node)) {
 		struct llext *ext = CONTAINER_OF(node, struct llext, _llext_list);
 
-		if (strncmp(ext->name, name, sizeof(ext->name)) == 0) {
+		if (strncmp(ext->name, name, LLEXT_MAX_NAME_LEN) == 0) {
 			k_mutex_unlock(&llext_lock);
 			return ext;
 		}
@@ -193,8 +193,9 @@ int llext_load(struct llext_loader *ldr, const char *name, struct llext **ext,
 		goto out;
 	}
 
-	strncpy((*ext)->name, name, sizeof((*ext)->name));
-	(*ext)->name[sizeof((*ext)->name) - 1] = '\0';
+	/* The (*ext)->name array is LLEXT_MAX_NAME_LEN + 1 bytes long */
+	strncpy((*ext)->name, name, LLEXT_MAX_NAME_LEN);
+	(*ext)->name[LLEXT_MAX_NAME_LEN] = '\0';
 	(*ext)->use_count++;
 
 	sys_slist_append(&_llext_list, &(*ext)->_llext_list);

subsys/llext/shell.c

@@ -128,18 +128,20 @@ static uint8_t llext_buf[CONFIG_LLEXT_SHELL_MAX_SIZE] __aligned(Z_KERNEL_STACK_O
 
 static int cmd_llext_load_hex(const struct shell *sh, size_t argc, char *argv[])
 {
-	char name[16];
-	size_t hex_len = strnlen(argv[2], CONFIG_LLEXT_SHELL_MAX_SIZE*2+1);
-	size_t bin_len = hex_len/2;
+	char *name = argv[1];
+	size_t hex_len = strlen(argv[2]);
 
-	if (bin_len > CONFIG_LLEXT_SHELL_MAX_SIZE) {
+	if (strlen(name) > LLEXT_MAX_NAME_LEN) {
+		shell_print(sh, "Extension name too long, max %d chars\n", LLEXT_MAX_NAME_LEN);
+		return -EINVAL;
+	}
+
+	if (hex_len > CONFIG_LLEXT_SHELL_MAX_SIZE*2) {
 		shell_print(sh, "Extension %d bytes too large to load, max %d bytes\n", hex_len/2,
 			    CONFIG_LLEXT_SHELL_MAX_SIZE);
 		return -ENOMEM;
 	}
 
-	strncpy(name, argv[1], sizeof(name));
-
 	size_t llext_buf_len = hex2bin(argv[2], hex_len, llext_buf, CONFIG_LLEXT_SHELL_MAX_SIZE);
 	struct llext_buf_loader buf_loader = LLEXT_BUF_LOADER(llext_buf, llext_buf_len);
 	struct llext_loader *ldr = &buf_loader.loader;