From: Marcus Glocker <marcus@nazgul.ch>
Subject: qciic: fix out-of-bounds read
To: tech@openbsd.org
Cc: Mark Kettenis <mark.kettenis@xs4all.nl>
Date: Sat, 23 May 2026 23:21:08 +0200

While working on a new driver, I've noticed that qciic wouldn't NULL
terminate a compatible string with a length of => 32 bytes, leading
to an out-of-bounds read later on:

"samsung,galaxybook-kbd-backlighth\^A\M^_$\M^@\M^?\M^?\M^?b" at iic3 addr 0x62 not configured

To fix this, the following diff does basically mimic apliic_bus_scan()
which works with malloc() for the compatible string instead.

As a side effect I also noticed that ia_namelen doesn't get set today,
which could cause issues to match an secondary fallback string.

After the diff:

"samsung,galaxybook-kbd-backlight" at iic3 addr 0x62 not configured

Ok?


Index: sys/dev/fdt/qciic_fdt.c
===================================================================
RCS file: /cvs/src/sys/dev/fdt/qciic_fdt.c,v
diff -u -p -u -p -r1.3 qciic_fdt.c
--- sys/dev/fdt/qciic_fdt.c	5 Jan 2026 20:06:15 -0000	1.3
+++ sys/dev/fdt/qciic_fdt.c	23 May 2026 21:04:40 -0000
@@ -18,6 +18,7 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/device.h>
+#include <sys/malloc.h>
 
 #include <machine/bus.h>
 #include <machine/intr.h>
@@ -308,39 +309,44 @@ qciic_fdt_bus_scan(struct device *self, 
 	int iba_node = *(int *)aux;
 	extern int iic_print(void *, const char *);
 	struct i2c_attach_args ia;
-	char name[32];
+	char *compat;
 	uint32_t reg[1];
 	int node;
+	int len;
 
 	for (node = OF_child(iba_node); node; node = OF_peer(node)) {
-		memset(name, 0, sizeof(name));
-		memset(reg, 0, sizeof(reg));
-
 		if (!OF_is_enabled(node))
 			continue;
 
-		if (OF_getprop(node, "compatible", name, sizeof(name)) == -1)
-			continue;
-		if (name[0] == '\0')
+		memset(reg, 0, sizeof(reg));
+		if (OF_getprop(node, "reg", &reg, sizeof(reg)) != sizeof(reg))
 			continue;
 
-		if (OF_getprop(node, "reg", &reg, sizeof(reg)) != sizeof(reg))
+		len = OF_getproplen(node, "compatible");
+		if (len <= 0)
 			continue;
 
+		compat = malloc(len, M_TEMP, M_WAITOK);
+		OF_getprop(node, "compatible", compat, len);
+
 		memset(&ia, 0, sizeof(ia));
 		ia.ia_tag = iba->iba_tag;
 		ia.ia_addr = bemtoh32(&reg[0]);
-		ia.ia_name = name;
+		ia.ia_name = compat;
+		ia.ia_namelen = len;
 		ia.ia_cookie = &node;
 		ia.ia_intr = &node;
 
 		/* Quirk for ihidev(4) */
-		if (strcmp(name, "hid-over-i2c") == 0) {
+		if (strcmp(compat, "hid-over-i2c") == 0) {
 			ia.ia_name = "ihidev";
+			ia.ia_namelen = 0;
 			ia.ia_size = OF_getpropint(node, "hid-descr-addr", 0);
-			ia.ia_cookie = name;
+			ia.ia_cookie = compat;
 		}
 
 		config_found(self, &ia, iic_print);
+
+		free(compat, M_TEMP, len);
 	}
 }