Re: udev breakages - was: Re: Need of an ".async_probe()" type of callback at driver's core - Was: Re: [PATCH] [media] drxk: change it to use request_firmware_nowait()

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 714 bytes --]

On Wed, Oct 3, 2012 at 10:09 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> +       if (!S_ISREG(inode->i_mode))
> +               return false;
> +       size = i_size_read(inode);
>
> Probably better to do vfs_getattr() and check mode and size in kstat; if
> it's sufficiently hot for that to hurt, we are fucked anyway.
>
> +               file = filp_open(path, O_RDONLY, 0);
> +               if (IS_ERR(file))
> +                       continue;
> +printk("from file '%s' ", path);
> +               success = fw_read_file_contents(file, fw);
> +               filp_close(file, NULL);
>
> fput(file), please.  We have enough misuses of filp_close() as it is...

Ok, like this?

                      Linus

[-- Attachment #2: patch.diff --]
[-- Type: application/octet-stream, Size: 2814 bytes --]

 drivers/base/firmware_class.c | 73 ++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 6e210802c37b..38feac4af991 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -21,6 +21,7 @@
 #include <linux/firmware.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
+#include <linux/file.h>
 #include <linux/list.h>
 #include <linux/async.h>
 #include <linux/pm.h>
@@ -33,6 +34,67 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
 MODULE_DESCRIPTION("Multi purpose firmware loading support");
 MODULE_LICENSE("GPL");
 
+/* Don't inline this: 'struct kstat' is biggish */
+static noinline long fw_file_size(struct file *file)
+{
+	struct kstat st;
+	if (vfs_getattr(file->f_path.mnt, file->f_path.dentry, &st))
+		return -1;
+	if (!S_ISREG(st.mode))
+		return -1;
+	if (st.size != (long)st.size)
+		return -1;
+	return st.size;
+}
+
+static bool fw_read_file_contents(struct file *file, struct firmware *fw)
+{
+	loff_t pos;
+	long size;
+	char *buf;
+
+	size = fw_file_size(file);
+	if (size < 0)
+		return false;
+	buf = vmalloc(size);
+	if (!buf)
+		return false;
+	pos = 0;
+	if (vfs_read(file, buf, size, &pos) != size) {
+		vfree(buf);
+		return false;
+	}
+	fw->data = buf;
+	fw->size = size;
+	return true;
+}
+
+static bool fw_get_filesystem_firmware(struct firmware *fw, const char *name)
+{
+	int i;
+	bool success = false;
+	const char *fw_path[] = { "/lib/firmware/update", "/firmware", "/lib/firmware" };
+	char *path = __getname();
+
+printk("Trying to load fw '%s' ", name);
+	for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
+		struct file *file;
+		snprintf(path, PATH_MAX, "%s/%s", fw_path[i], name);
+
+		file = filp_open(path, O_RDONLY, 0);
+		if (IS_ERR(file))
+			continue;
+printk("from file '%s' ", path);
+		success = fw_read_file_contents(file, fw);
+		fput(file);
+		if (success)
+			break;
+	}
+printk(" %s.\n", success ? "Ok" : "failed");
+	__putname(path);
+	return success;
+}
+
 /* Builtin firmware support */
 
 #ifdef CONFIG_FW_LOADER
@@ -346,7 +408,11 @@ static ssize_t firmware_loading_show(struct device *dev,
 /* firmware holds the ownership of pages */
 static void firmware_free_data(const struct firmware *fw)
 {
-	WARN_ON(!fw->priv);
+	/* Loaded directly? */
+	if (!fw->priv) {
+		vfree(fw->data);
+		return;
+	}
 	fw_free_buf(fw->priv);
 }
 
@@ -709,6 +775,11 @@ _request_firmware_prepare(const struct firmware **firmware_p, const char *name,
 		return NULL;
 	}
 
+	if (fw_get_filesystem_firmware(firmware, name)) {
+		dev_dbg(device, "firmware: direct-loading firmware %s\n", name);
+		return NULL;
+	}
+
 	ret = fw_lookup_and_allocate_buf(name, &fw_cache, &buf);
 	if (!ret)
 		fw_priv = fw_create_instance(firmware, name, device,