[PATCH v6 0/2] firmware_loader: allow firmware_class.path to take multiple paths

[PATCH v6 0/2] firmware_loader: allow firmware_class.path to take multiple paths

[Jeff Layton]

This is something Michal had asked for last year, and I just got around
to implementing. This version is just a minor cleanup for
reviewability's sake. I also renamed the search= option to search_path=.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Changes in v6:
- Add fw_search_unescape() to make the search path parsing more readable
- Rename search= to search_path=
- Link to v5: https://lore.kernel.org/r/20260323-fw-path-v5-0-e88b2fe145f3@kernel.org

Changes in v5:
- Increase search= string length from 256 to 4096
- Preprocess search= path when it's set rather than on every firmware load
- Add selftests for search= functionality
- Link to v4: https://lore.kernel.org/r/20260320-fw-path-v4-1-7547e2f0dc64@kernel.org

Changes in v4:
- Move search path to new search= option that is tried after path=
- Link to v3: https://lore.kernel.org/r/20260318-fw-path-v3-1-a701a08bc025@kernel.org

Changes in v3:
- Allow '\' to escape a literal ':' or '\' in the string
- Link to v2: https://lore.kernel.org/r/20260318-fw-path-v2-1-8a106eb91eb4@kernel.org

Changes in v2:
- switch to using ':' as path delimiter
- Link to v1: https://lore.kernel.org/r/20260318-fw-path-v1-1-7884d9bf618f@kernel.org

---
Jeff Layton (2):
      firmware_loader: add search_path= module option for multi-path firmware lookup
      selftests/firmware: add search path test for firmware_class.search_path=

 drivers/base/firmware_loader/main.c           | 298 +++++++++++++++++++-------
 tools/testing/selftests/firmware/Makefile     |   2 +-
 tools/testing/selftests/firmware/fw_search.sh | 222 +++++++++++++++++++
 3 files changed, 445 insertions(+), 77 deletions(-)
---
base-commit: c369299895a591d96745d6492d4888259b004a9e
change-id: 20260317-fw-path-a094c30259a5

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH v6 1/2] firmware_loader: add search_path= module option for multi-path firmware lookup

[Jeff Layton]

Refactor fw_get_filesystem_firmware() by extracting the per-path
firmware loading logic into a new fw_try_firmware_path() helper.

Add a new firmware_class.search_path= module option that accepts a
':'-separated list of firmware search directories. The input is
preprocessed at set time (boot or sysfs write) into a NUL-separated
sequence of paths, avoiding per-load parsing overhead. Backslash
escapes are supported: '\:' for literal ':', '\\' for literal '\'.

The firmware lookup order is:

  1. firmware_class.path= (single legacy path)
  2. firmware_class.search_path= (colon-separated paths)
  3. Built-in default paths (/lib/firmware/updates/..., /lib/firmware)

Example:

  firmware_class.search_path=/custom/path1:/custom/path2

Suggested-by: Michal Grzedzicki <mge@meta.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 drivers/base/firmware_loader/main.c | 298 +++++++++++++++++++++++++++---------
 1 file changed, 222 insertions(+), 76 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index a11b30dda23be563bd55f25474ceff2153ddd667..06f9530b3eba22187f34416fcd00c1cab8c79291 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -469,8 +469,9 @@ static int fw_decompress_xz(struct device *dev, struct fw_priv *fw_priv,
 
 /* direct firmware loading support */
 static char fw_path_para[256];
+static char fw_search_para[4096];
+static int fw_search_len;
 static const char * const fw_path[] = {
-	fw_path_para,
 	"/lib/firmware/updates/" UTS_RELEASE,
 	"/lib/firmware/updates",
 	"/lib/firmware/" UTS_RELEASE,
@@ -485,6 +486,184 @@ static const char * const fw_path[] = {
 module_param_string(path, fw_path_para, sizeof(fw_path_para), 0644);
 MODULE_PARM_DESC(path, "customized firmware image search path with a higher priority than default path");
 
+/*
+ * fw_search_unescape - copy src to dst, processing backslash escapes
+ *
+ * Handles '\:' (literal ':') and '\\' (literal '\'). Returns the number
+ * of bytes written (not counting the trailing NUL).
+ */
+static int fw_search_unescape(char *dst, int dst_size, const char *src, int src_len)
+{
+	int i, len = 0;
+
+	for (i = 0; i < src_len && len < dst_size - 1; i++) {
+		if (src[i] == '\\' && i + 1 < src_len &&
+		    (src[i + 1] == ':' || src[i + 1] == '\\'))
+			i++;
+		dst[len++] = src[i];
+	}
+	dst[len] = '\0';
+	return len;
+}
+
+/*
+ * fw_search_set - preprocess a colon-separated search path string
+ *
+ * Converts the input into a NUL-separated sequence of paths stored in
+ * fw_search_para, with fw_search_len tracking the total used length.
+ * Backslash escapes '\:' (literal ':') and '\\' (literal '\').
+ * Trailing newlines on each component are stripped.
+ */
+static int fw_search_set(const char *val, const struct kernel_param *kp)
+{
+	int len = 0;
+
+	if (!val) {
+		fw_search_para[0] = '\0';
+		fw_search_len = 0;
+		return 0;
+	}
+
+	while (*val) {
+		const char *sep;
+		int comp_len, remaining;
+
+		/* find the next unescaped ':' or end of string */
+		for (sep = val; *sep; sep++) {
+			if (*sep == '\\' && (sep[1] == ':' || sep[1] == '\\'))
+				sep++;
+			else if (*sep == ':')
+				break;
+		}
+
+		remaining = sizeof(fw_search_para) - len - 1;
+		comp_len = fw_search_unescape(fw_search_para + len, remaining,
+					       val, sep - val);
+
+		/* strip trailing newline */
+		if (comp_len > 0 && fw_search_para[len + comp_len - 1] == '\n')
+			comp_len--;
+
+		/* only record non-empty components */
+		if (comp_len > 0) {
+			len += comp_len;
+			fw_search_para[len++] = '\0';
+		}
+
+		val = *sep ? sep + 1 : sep;
+	}
+
+	/* ensure NUL termination even when empty */
+	fw_search_para[len] = '\0';
+	fw_search_len = len;
+
+	return 0;
+}
+
+/*
+ * fw_search_get - reconstruct colon-separated string for sysfs reads
+ */
+static int fw_search_get(char *buffer, const struct kernel_param *kp)
+{
+	const char *p;
+	int pos = 0;
+
+	p = fw_search_para;
+	while (p < fw_search_para + fw_search_len) {
+		int slen = strlen(p);
+
+		if (!slen)
+			break;
+		if (pos > 0)
+			buffer[pos++] = ':';
+		memcpy(buffer + pos, p, slen);
+		pos += slen;
+		p += slen + 1;
+	}
+	buffer[pos] = '\0';
+	return pos;
+}
+
+static const struct kernel_param_ops fw_search_ops = {
+	.set = fw_search_set,
+	.get = fw_search_get,
+};
+module_param_cb(search_path, &fw_search_ops, NULL, 0644);
+MODULE_PARM_DESC(search_path, "colon-separated list of firmware search paths, tried after path= (use '\\:' for literal ':', '\\\\' for literal '\\')");
+
+static int
+fw_try_firmware_path(struct device *device, struct fw_priv *fw_priv,
+		     const char *suffix,
+		     int (*decompress)(struct device *dev,
+				       struct fw_priv *fw_priv,
+				       size_t in_size,
+				       const void *in_buffer),
+		     const char *dir, int dirlen,
+		     char *path, void **bufp, size_t msize)
+{
+	size_t file_size = 0;
+	size_t *file_size_ptr = NULL;
+	size_t size;
+	int len, rc;
+
+	len = snprintf(path, PATH_MAX, "%.*s/%s%s",
+		       dirlen, dir, fw_priv->fw_name, suffix);
+	if (len >= PATH_MAX)
+		return -ENAMETOOLONG;
+
+	fw_priv->size = 0;
+
+	/*
+	 * The total file size is only examined when doing a partial
+	 * read; the "full read" case needs to fail if the whole
+	 * firmware was not completely loaded.
+	 */
+	if ((fw_priv->opt_flags & FW_OPT_PARTIAL) && *bufp)
+		file_size_ptr = &file_size;
+
+	/* load firmware files from the mount namespace of init */
+	rc = kernel_read_file_from_path_initns(path, fw_priv->offset,
+					       bufp, msize,
+					       file_size_ptr,
+					       READING_FIRMWARE);
+	if (rc < 0) {
+		if (!(fw_priv->opt_flags & FW_OPT_NO_WARN)) {
+			if (rc != -ENOENT)
+				dev_warn(device,
+					 "loading %s failed with error %d\n",
+					 path, rc);
+			else
+				dev_dbg(device,
+					"loading %s failed for no such file or directory.\n",
+					path);
+		}
+		return rc;
+	}
+	size = rc;
+
+	dev_dbg(device, "Loading firmware from %s\n", path);
+	if (decompress) {
+		dev_dbg(device, "f/w decompressing %s\n",
+			fw_priv->fw_name);
+		rc = decompress(device, fw_priv, size, *bufp);
+		/* discard the superfluous original content */
+		vfree(*bufp);
+		*bufp = NULL;
+		if (rc) {
+			fw_free_paged_buf(fw_priv);
+			return rc;
+		}
+	} else {
+		dev_dbg(device, "direct-loading %s\n",
+			fw_priv->fw_name);
+		if (!fw_priv->data)
+			fw_priv->data = *bufp;
+		fw_priv->size = size;
+	}
+	fw_state_done(fw_priv);
+	return 0;
+}
+
 static int
 fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 			   const char *suffix,
@@ -493,10 +672,9 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 					     size_t in_size,
 					     const void *in_buffer))
 {
-	size_t size;
-	int i, len, maxlen = 0;
+	int i;
 	int rc = -ENOENT;
-	char *path, *nt = NULL;
+	char *path;
 	size_t msize = INT_MAX;
 	void *buffer = NULL;
 
@@ -511,83 +689,51 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 		return -ENOMEM;
 
 	wait_for_initramfs();
-	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
-		size_t file_size = 0;
-		size_t *file_size_ptr = NULL;
-
-		/* skip the unset customized path */
-		if (!fw_path[i][0])
-			continue;
-
-		/* strip off \n from customized path */
-		maxlen = strlen(fw_path[i]);
-		if (i == 0) {
-			nt = strchr(fw_path[i], '\n');
-			if (nt)
-				maxlen = nt - fw_path[i];
-		}
 
-		len = snprintf(path, PATH_MAX, "%.*s/%s%s",
-			       maxlen, fw_path[i],
-			       fw_priv->fw_name, suffix);
-		if (len >= PATH_MAX) {
-			rc = -ENAMETOOLONG;
-			break;
-		}
+	/* Try the customized path first */
+	if (fw_path_para[0]) {
+		int dirlen = strlen(fw_path_para);
 
-		fw_priv->size = 0;
+		/* strip trailing newline */
+		if (fw_path_para[dirlen - 1] == '\n')
+			dirlen--;
 
-		/*
-		 * The total file size is only examined when doing a partial
-		 * read; the "full read" case needs to fail if the whole
-		 * firmware was not completely loaded.
-		 */
-		if ((fw_priv->opt_flags & FW_OPT_PARTIAL) && buffer)
-			file_size_ptr = &file_size;
-
-		/* load firmware files from the mount namespace of init */
-		rc = kernel_read_file_from_path_initns(path, fw_priv->offset,
-						       &buffer, msize,
-						       file_size_ptr,
-						       READING_FIRMWARE);
-		if (rc < 0) {
-			if (!(fw_priv->opt_flags & FW_OPT_NO_WARN)) {
-				if (rc != -ENOENT)
-					dev_warn(device,
-						 "loading %s failed with error %d\n",
-						 path, rc);
-				else
-					dev_dbg(device,
-						"loading %s failed for no such file or directory.\n",
-						path);
-			}
-			continue;
-		}
-		size = rc;
-		rc = 0;
-
-		dev_dbg(device, "Loading firmware from %s\n", path);
-		if (decompress) {
-			dev_dbg(device, "f/w decompressing %s\n",
-				fw_priv->fw_name);
-			rc = decompress(device, fw_priv, size, buffer);
-			/* discard the superfluous original content */
-			vfree(buffer);
-			buffer = NULL;
-			if (rc) {
-				fw_free_paged_buf(fw_priv);
-				continue;
-			}
-		} else {
-			dev_dbg(device, "direct-loading %s\n",
-				fw_priv->fw_name);
-			if (!fw_priv->data)
-				fw_priv->data = buffer;
-			fw_priv->size = size;
+		rc = fw_try_firmware_path(device, fw_priv, suffix, decompress,
+					  fw_path_para, dirlen,
+					  path, &buffer, msize);
+		if (!rc)
+			goto done;
+	}
+
+	/* Try each preprocessed NUL-separated path in fw_search_para */
+	if (fw_search_len > 0) {
+		const char *p = fw_search_para;
+
+		while (p < fw_search_para + fw_search_len) {
+			int dirlen = strlen(p);
+
+			if (!dirlen)
+				break;
+			rc = fw_try_firmware_path(device, fw_priv,
+						  suffix, decompress,
+						  p, dirlen,
+						  path, &buffer, msize);
+			if (!rc)
+				goto done;
+			p += dirlen + 1;
 		}
-		fw_state_done(fw_priv);
-		break;
 	}
+
+	/* Try default firmware paths */
+	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+		rc = fw_try_firmware_path(device, fw_priv, suffix, decompress,
+					  fw_path[i], strlen(fw_path[i]),
+					  path, &buffer, msize);
+		if (!rc)
+			break;
+	}
+
+done:
 	__putname(path);
 
 	return rc;

-- 
2.53.0

[PATCH v6 2/2] selftests/firmware: add search path test for firmware_class.search_path=

[Jeff Layton]

Add fw_search.sh, a new selftest that validates the firmware_class.search_path=
module parameter using the existing test_firmware module's sysfs trigger
interface.

The test covers:
  - Firmware found in first/second/third search directory
  - Firmware not found in any search path
  - path= takes priority over search_path=
  - Empty search_path= does not break firmware loading
  - Sysfs readback matches what was written
  - Escaped colon (\:) in directory name
  - Escaped backslash (\\) in directory name
  - Escaped colon combined with multiple search paths

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 tools/testing/selftests/firmware/Makefile     |   2 +-
 tools/testing/selftests/firmware/fw_search.sh | 222 ++++++++++++++++++++++++++
 2 files changed, 223 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/firmware/Makefile b/tools/testing/selftests/firmware/Makefile
index 7992969deaa2737ff2a033ffe60136b84ea2f3f0..42e5bd72886b3cd8f62ae42d53c1554e8a1b331c 100644
--- a/tools/testing/selftests/firmware/Makefile
+++ b/tools/testing/selftests/firmware/Makefile
@@ -3,7 +3,7 @@
 CFLAGS = -Wall \
          -O2
 
-TEST_PROGS := fw_run_tests.sh
+TEST_PROGS := fw_run_tests.sh fw_search.sh
 TEST_FILES := fw_fallback.sh fw_filesystem.sh fw_upload.sh fw_lib.sh
 TEST_GEN_FILES := fw_namespace
 
diff --git a/tools/testing/selftests/firmware/fw_search.sh b/tools/testing/selftests/firmware/fw_search.sh
new file mode 100755
index 0000000000000000000000000000000000000000..19a23284c31104c3bad3044a910c50aa71a2a766
--- /dev/null
+++ b/tools/testing/selftests/firmware/fw_search.sh
@@ -0,0 +1,222 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+#
+# Test the firmware_class.search_path= module parameter, which allows
+# specifying multiple colon-separated firmware search directories.
+
+set -e
+
+TEST_REQS_FW_SYSFS_FALLBACK="no"
+TEST_REQS_FW_SET_CUSTOM_PATH="no"
+TEST_DIR=$(dirname $0)
+source $TEST_DIR/fw_lib.sh
+
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+SEARCH_SYSFS="/sys/module/firmware_class/parameters/search_path"
+PATH_SYSFS="/sys/module/firmware_class/parameters/path"
+
+check_mods
+check_setup
+verify_reqs
+
+if [ ! -f "$SEARCH_SYSFS" ]; then
+	echo "$0: search_path= parameter not available, skipping"
+	exit $ksft_skip
+fi
+
+# Save original values
+OLD_SEARCH="$(cat $SEARCH_SYSFS)"
+OLD_PATH="$(cat $PATH_SYSFS)"
+
+# Create temp directories for firmware
+FWDIR1=$(mktemp -d)
+FWDIR2=$(mktemp -d)
+FWDIR3=$(mktemp -d)
+
+FW_NAME="test-search-fw.bin"
+FW_CONTENT1="SEARCH_PATH_1"
+FW_CONTENT2="SEARCH_PATH_2"
+FW_CONTENT3="SEARCH_PATH_3"
+
+DIR=/sys/devices/virtual/misc/test_firmware
+
+cleanup()
+{
+	# Restore original values
+	if [ "$OLD_PATH" = "" ]; then
+		printf '\000' >$PATH_SYSFS
+	else
+		echo -n "$OLD_PATH" >$PATH_SYSFS
+	fi
+	if [ "$OLD_SEARCH" = "" ]; then
+		printf '\000' >$SEARCH_SYSFS
+	else
+		echo -n "$OLD_SEARCH" >$SEARCH_SYSFS
+	fi
+	rm -rf "$FWDIR1" "$FWDIR2" "$FWDIR3"
+}
+trap cleanup EXIT
+
+# Clear path= so search_path= is consulted
+printf '\000' >$PATH_SYSFS
+
+# Test 1: firmware found in first search path
+echo -n "$FW_CONTENT1" >"$FWDIR1/$FW_NAME"
+echo -n "$FWDIR1:$FWDIR2" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 1" >&2
+	exit 1
+fi
+if ! diff -q "$FWDIR1/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - firmware content mismatch in test 1" >&2
+	exit 1
+fi
+echo "$0: search path - first directory: OK"
+
+# Test 2: firmware found in second search path (not in first)
+rm -f "$FWDIR1/$FW_NAME"
+echo -n "$FW_CONTENT2" >"$FWDIR2/$FW_NAME"
+echo -n "$FWDIR1:$FWDIR2" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 2" >&2
+	exit 1
+fi
+if ! diff -q "$FWDIR2/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - firmware content mismatch in test 2" >&2
+	exit 1
+fi
+echo "$0: search path - second directory: OK"
+
+# Test 3: firmware not found in any search path
+rm -f "$FWDIR2/$FW_NAME"
+echo -n "$FWDIR1:$FWDIR2" >$SEARCH_SYSFS
+
+if echo -n "nonexistent-$FW_NAME" >$DIR/trigger_request 2>/dev/null; then
+	echo "$0: FAIL - firmware should not have been found in test 3" >&2
+	exit 1
+fi
+echo "$0: search path - not found: OK"
+
+# Test 4: path= takes priority over search_path=
+echo -n "$FW_CONTENT1" >"$FWDIR1/$FW_NAME"
+echo -n "$FW_CONTENT2" >"$FWDIR2/$FW_NAME"
+echo -n "$FWDIR1" >$PATH_SYSFS
+echo -n "$FWDIR2" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 4" >&2
+	exit 1
+fi
+if ! diff -q "$FWDIR1/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - path= should take priority over search_path= in test 4" >&2
+	exit 1
+fi
+echo "$0: search path - path= priority over search_path=: OK"
+
+# Clear path= again for remaining tests
+printf '\000' >$PATH_SYSFS
+
+# Test 5: three search paths, firmware in third
+rm -f "$FWDIR1/$FW_NAME" "$FWDIR2/$FW_NAME"
+echo -n "$FW_CONTENT3" >"$FWDIR3/$FW_NAME"
+echo -n "$FWDIR1:$FWDIR2:$FWDIR3" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 5" >&2
+	exit 1
+fi
+if ! diff -q "$FWDIR3/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - firmware content mismatch in test 5" >&2
+	exit 1
+fi
+echo "$0: search path - third directory: OK"
+
+# Test 6: empty search_path= should not break anything
+rm -f "$FWDIR3/$FW_NAME"
+printf '\000' >$SEARCH_SYSFS
+
+if echo -n "nonexistent-$FW_NAME" >$DIR/trigger_request 2>/dev/null; then
+	echo "$0: FAIL - empty search_path= should not find firmware" >&2
+	exit 1
+fi
+echo "$0: search path - empty search_path=: OK"
+
+# Test 7: verify sysfs readback matches what was written
+echo -n "$FWDIR1:$FWDIR2:$FWDIR3" >$SEARCH_SYSFS
+READBACK="$(cat $SEARCH_SYSFS)"
+EXPECTED="$FWDIR1:$FWDIR2:$FWDIR3"
+if [ "$READBACK" != "$EXPECTED" ]; then
+	echo "$0: FAIL - sysfs readback mismatch: '$READBACK' != '$EXPECTED'" >&2
+	exit 1
+fi
+echo "$0: search path - sysfs readback: OK"
+
+# Test 8: escaped colon in directory name (\: -> literal ':')
+FWDIR_COLON=$(mktemp -d)/fw:dir
+mkdir -p "$FWDIR_COLON"
+echo -n "$FW_CONTENT1" >"$FWDIR_COLON/$FW_NAME"
+# Write the path with the colon escaped as \:
+ESCAPED_COLON="${FWDIR_COLON//:/\\:}"
+echo -n "$ESCAPED_COLON" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 8" >&2
+	rm -rf "$(dirname "$FWDIR_COLON")"
+	exit 1
+fi
+if ! diff -q "$FWDIR_COLON/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - firmware content mismatch in test 8" >&2
+	rm -rf "$(dirname "$FWDIR_COLON")"
+	exit 1
+fi
+rm -rf "$(dirname "$FWDIR_COLON")"
+echo "$0: search path - escaped colon in directory: OK"
+
+# Test 9: escaped backslash in directory name (\\ -> literal '\')
+FWDIR_BS=$(mktemp -d)/fw\\dir
+mkdir -p "$FWDIR_BS"
+echo -n "$FW_CONTENT2" >"$FWDIR_BS/$FW_NAME"
+# Write the path with backslashes escaped as \\
+ESCAPED_BS="${FWDIR_BS//\\/\\\\}"
+echo -n "$ESCAPED_BS" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 9" >&2
+	rm -rf "$(dirname "$FWDIR_BS")"
+	exit 1
+fi
+if ! diff -q "$FWDIR_BS/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - firmware content mismatch in test 9" >&2
+	rm -rf "$(dirname "$FWDIR_BS")"
+	exit 1
+fi
+rm -rf "$(dirname "$FWDIR_BS")"
+echo "$0: search path - escaped backslash in directory: OK"
+
+# Test 10: escaped colon with multiple search paths
+FWDIR_COLON2=$(mktemp -d)/has:colon
+mkdir -p "$FWDIR_COLON2"
+echo -n "$FW_CONTENT3" >"$FWDIR_COLON2/$FW_NAME"
+ESCAPED_COLON2="${FWDIR_COLON2//:/\\:}"
+# First path is normal (no firmware), second has escaped colon (has firmware)
+echo -n "$FWDIR1:$ESCAPED_COLON2" >$SEARCH_SYSFS
+
+if ! echo -n "$FW_NAME" >$DIR/trigger_request; then
+	echo "$0: FAIL - could not trigger request for test 10" >&2
+	rm -rf "$(dirname "$FWDIR_COLON2")"
+	exit 1
+fi
+if ! diff -q "$FWDIR_COLON2/$FW_NAME" /dev/test_firmware >/dev/null; then
+	echo "$0: FAIL - firmware content mismatch in test 10" >&2
+	rm -rf "$(dirname "$FWDIR_COLON2")"
+	exit 1
+fi
+rm -rf "$(dirname "$FWDIR_COLON2")"
+echo "$0: search path - escaped colon with multiple paths: OK"
+
+echo "$0: all search path tests passed"
+exit 0

-- 
2.53.0

Re: [PATCH v6 0/2] firmware_loader: allow firmware_class.path to take multiple paths

[Greg Kroah-Hartman]

On Wed, Apr 01, 2026 at 09:43:26AM -0400, Jeff Layton wrote:
> This is something Michal had asked for last year, and I just got around
> to implementing. This version is just a minor cleanup for
> reviewability's sake. I also renamed the search= option to search_path=.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---

While it is still experimental, and might not be fully correct, you
might want to look here:
	https://sashiko.dev/#/patchset/20260401-fw-path-v6-0-4ebe70441839%40kernel.org
to see a few potential issues for this patchset (notibly at least the
bash stuff)

thanks,

greg k-h

Re: [PATCH v6 0/2] firmware_loader: allow firmware_class.path to take multiple paths

[Jeff Layton]

On Thu, 2026-04-02 at 06:13 +0200, Greg Kroah-Hartman wrote:
> On Wed, Apr 01, 2026 at 09:43:26AM -0400, Jeff Layton wrote:
> > This is something Michal had asked for last year, and I just got around
> > to implementing. This version is just a minor cleanup for
> > reviewability's sake. I also renamed the search= option to search_path=.
> > 
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> 
> While it is still experimental, and might not be fully correct, you
> might want to look here:
> 	https://sashiko.dev/#/patchset/20260401-fw-path-v6-0-4ebe70441839%40kernel.org
> to see a few potential issues for this patchset (notibly at least the
> bash stuff)
> 

I'll plan to fix up the bash ones.

This race in the C review looks legit too:

------------------8<-------------------
-
> +	/* Try the customized path first */
> +	if (fw_path_para[0]) {
> +		int dirlen = strlen(fw_path_para);
>  
> -		fw_priv->size = 0;
> +		/* strip trailing newline */
> +		if (fw_path_para[dirlen - 1] == '\n')
> +			dirlen--;

Can a concurrent sysfs write cause an out-of-bounds read here?

If a sysfs write clears the string (e.g., by echoing an empty string to the
path parameter) immediately after the if (fw_path_para[0]) check,
strlen(fw_path_para) could return 0.

This would cause the code to evaluate fw_path_para[-1] == '\n', accessing
memory before the array bounds.
------------------8<-------------------

My reviews with Claude noticed the first race too, but I had concluded
that it was benign (you'd get a failed firmware load, but it shouldn't
crash).

I think that the simplest fix would be to just add a global rwsem
around the get/set routines. I'll plan to send a v7.

Thanks for the pointer!
-- 
Jeff Layton <jlayton@kernel.org>