public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] firmware_loader: add search= module option for multi-path firmware lookup
@ 2026-03-20 15:32 Jeff Layton
  2026-03-20 15:59 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Layton @ 2026-03-20 15:32 UTC (permalink / raw)
  To: Luis Chamberlain, Russ Weight, Danilo Krummrich,
	Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: Michal Grzedzicki, driver-core, linux-kernel, 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= module option that accepts a
':'-separated list of firmware search directories. The firmware
lookup order is:

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

A backslash can be used as an escape character, allowing a literal
':' ('\:') or literal '\' ('\\') to be embedded in a pathname.

Example:

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

Suggested-by: Michal Grzedzicki <mge@meta.com>
Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
This version adds a new module option called "search=" to specify an
(escapable) search path. The search= path is tried after the original
(unescaped) path= string, but before the hardcoded search path.
---
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
---
 drivers/base/firmware_loader/main.c | 224 ++++++++++++++++++++++++------------
 1 file changed, 150 insertions(+), 74 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index a11b30dda23be563bd55f25474ceff2153ddd667..0f58d996d3807a1f57dc924adbe657360fdfac1e 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -469,8 +469,8 @@ 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[256];
 static const char * const fw_path[] = {
-	fw_path_para,
 	"/lib/firmware/updates/" UTS_RELEASE,
 	"/lib/firmware/updates",
 	"/lib/firmware/" UTS_RELEASE,
@@ -485,6 +485,82 @@ 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");
 
+module_param_string(search, fw_search_para, sizeof(fw_search_para), 0644);
+MODULE_PARM_DESC(search, "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 +569,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 +586,84 @@ 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 single 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;
+		rc = fw_try_firmware_path(device, fw_priv, suffix, decompress,
+					  fw_path_para, dirlen,
+					  path, &buffer, msize);
+		if (!rc)
+			goto done;
+	}
+
+	/* Try each ':'-separated path in fw_search_para */
+	if (fw_search_para[0]) {
+		const char *start = fw_search_para;
+		const char *p;
+		char *dir;
+
+		dir = __getname();
+		if (!dir) {
+			rc = -ENOMEM;
+			goto done;
 		}
-		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;
+
+		while (*start) {
+			int dirlen = 0;
+
+			/*
+			 * Parse the next path component. A backslash
+			 * escapes the following character, allowing
+			 * literal ':' ('\:') and literal '\' ('\\')
+			 * to be embedded in a path.
+			 */
+			for (p = start; *p && *p != ':'; p++) {
+				if (p[0] == '\\' && (p[1] == ':' || p[1] == '\\'))
+					p++;
+				if (dirlen < PATH_MAX - 1)
+					dir[dirlen++] = *p;
 			}
-		} else {
-			dev_dbg(device, "direct-loading %s\n",
-				fw_priv->fw_name);
-			if (!fw_priv->data)
-				fw_priv->data = buffer;
-			fw_priv->size = size;
+
+			/* strip trailing newline */
+			if (dirlen > 0 && dir[dirlen - 1] == '\n')
+				dirlen--;
+
+			if (dirlen > 0) {
+				dir[dirlen] = '\0';
+				rc = fw_try_firmware_path(device, fw_priv,
+							  suffix, decompress,
+							  dir, dirlen,
+							  path, &buffer,
+							  msize);
+				if (!rc) {
+					__putname(dir);
+					goto done;
+				}
+			}
+
+			if (!*p)
+				break;
+			start = p + 1;
 		}
-		fw_state_done(fw_priv);
-		break;
+		__putname(dir);
 	}
+
+	/* 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;

---
base-commit: 2d1373e4246da3b58e1df058374ed6b101804e07
change-id: 20260317-fw-path-a094c30259a5

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] firmware_loader: add search= module option for multi-path firmware lookup
  2026-03-20 15:32 [PATCH v4] firmware_loader: add search= module option for multi-path firmware lookup Jeff Layton
@ 2026-03-20 15:59 ` Greg Kroah-Hartman
  2026-03-20 16:23   ` Jeff Layton
  0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2026-03-20 15:59 UTC (permalink / raw)
  To: Jeff Layton
  Cc: Luis Chamberlain, Russ Weight, Danilo Krummrich,
	Rafael J. Wysocki, Michal Grzedzicki, driver-core, linux-kernel

On Fri, Mar 20, 2026 at 11:32:29AM -0400, Jeff Layton wrote:
> 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= module option that accepts a
> ':'-separated list of firmware search directories. The firmware
> lookup order is:
> 
>   1. firmware_class.path= (single legacy path)
>   2. firmware_class.search= (colon-separated paths)
>   3. Built-in default paths (/lib/firmware/updates/..., /lib/firmware)
> 
> A backslash can be used as an escape character, allowing a literal
> ':' ('\:') or literal '\' ('\\') to be embedded in a pathname.
> 
> Example:
> 
>   firmware_class.search=/custom/path1:/custom/path2
> 
> Suggested-by: Michal Grzedzicki <mge@meta.com>
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
> This version adds a new module option called "search=" to specify an
> (escapable) search path. The search= path is tried after the original
> (unescaped) path= string, but before the hardcoded search path.
> ---
> 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
> ---
>  drivers/base/firmware_loader/main.c | 224 ++++++++++++++++++++++++------------
>  1 file changed, 150 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> index a11b30dda23be563bd55f25474ceff2153ddd667..0f58d996d3807a1f57dc924adbe657360fdfac1e 100644
> --- a/drivers/base/firmware_loader/main.c
> +++ b/drivers/base/firmware_loader/main.c
> @@ -469,8 +469,8 @@ 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[256];
>  static const char * const fw_path[] = {
> -	fw_path_para,
>  	"/lib/firmware/updates/" UTS_RELEASE,
>  	"/lib/firmware/updates",
>  	"/lib/firmware/" UTS_RELEASE,
> @@ -485,6 +485,82 @@ 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");

Change this to say "single directory location" or something like that?

>  
> +module_param_string(search, fw_search_para, sizeof(fw_search_para), 0644);

"search_path"?

Or "path_search"?

Naming is hard :(

> +MODULE_PARM_DESC(search, "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 +569,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 +586,84 @@ 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 single 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--;

Why don't we just do all the "parsing" and stripping at startup (and
when a new option is written), so we don't have to do it each and every
time we search for a firmware image?  That would put all of the
"cleanup" logic in one place and reduce the churn a bit.

Yes, it would require another local variable, but that should be fine.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] firmware_loader: add search= module option for multi-path firmware lookup
  2026-03-20 15:59 ` Greg Kroah-Hartman
@ 2026-03-20 16:23   ` Jeff Layton
  0 siblings, 0 replies; 3+ messages in thread
From: Jeff Layton @ 2026-03-20 16:23 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Luis Chamberlain, Russ Weight, Danilo Krummrich,
	Rafael J. Wysocki, Michal Grzedzicki, driver-core, linux-kernel

On Fri, 2026-03-20 at 16:59 +0100, Greg Kroah-Hartman wrote:
> On Fri, Mar 20, 2026 at 11:32:29AM -0400, Jeff Layton wrote:
> > 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= module option that accepts a
> > ':'-separated list of firmware search directories. The firmware
> > lookup order is:
> > 
> >   1. firmware_class.path= (single legacy path)
> >   2. firmware_class.search= (colon-separated paths)
> >   3. Built-in default paths (/lib/firmware/updates/..., /lib/firmware)
> > 
> > A backslash can be used as an escape character, allowing a literal
> > ':' ('\:') or literal '\' ('\\') to be embedded in a pathname.
> > 
> > Example:
> > 
> >   firmware_class.search=/custom/path1:/custom/path2
> > 
> > Suggested-by: Michal Grzedzicki <mge@meta.com>
> > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > ---
> > This version adds a new module option called "search=" to specify an
> > (escapable) search path. The search= path is tried after the original
> > (unescaped) path= string, but before the hardcoded search path.
> > ---
> > 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
> > ---
> >  drivers/base/firmware_loader/main.c | 224 ++++++++++++++++++++++++------------
> >  1 file changed, 150 insertions(+), 74 deletions(-)
> > 
> > diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
> > index a11b30dda23be563bd55f25474ceff2153ddd667..0f58d996d3807a1f57dc924adbe657360fdfac1e 100644
> > --- a/drivers/base/firmware_loader/main.c
> > +++ b/drivers/base/firmware_loader/main.c
> > @@ -469,8 +469,8 @@ 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[256];
> >  static const char * const fw_path[] = {
> > -	fw_path_para,
> >  	"/lib/firmware/updates/" UTS_RELEASE,
> >  	"/lib/firmware/updates",
> >  	"/lib/firmware/" UTS_RELEASE,
> > @@ -485,6 +485,82 @@ 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");
> 
> Change this to say "single directory location" or something like that?
> 
> >  
> > +module_param_string(search, fw_search_para, sizeof(fw_search_para), 0644);
> 
> "search_path"?
> 
> Or "path_search"?
> 
> Naming is hard :(
> 

Indeed. I'm fine with a different name if that's the consensus.

> > +MODULE_PARM_DESC(search, "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 +569,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 +586,84 @@ 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 single 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--;
> 
> Why don't we just do all the "parsing" and stripping at startup (and
> when a new option is written), so we don't have to do it each and every
> time we search for a firmware image?  That would put all of the
> "cleanup" logic in one place and reduce the churn a bit.
> 
> Yes, it would require another local variable, but that should be fine.
> 

Ok, I'll see what I can do.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-20 16:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-20 15:32 [PATCH v4] firmware_loader: add search= module option for multi-path firmware lookup Jeff Layton
2026-03-20 15:59 ` Greg Kroah-Hartman
2026-03-20 16:23   ` Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox