Re: Is request_firmware() really safe to call in resume callback when /usr/lib/firmware is on btrfs?

[Luis Chamberlain <mcgrof@kernel.org>]

On Sat, Apr 03, 2021 at 08:25:38PM +0000, Luis Chamberlain wrote:
> So creating say 1000 random files in /lib/firmware on a freshly created
> btrfs partition helps reproduce:
> 
> mkfs.btrfs /dev/whatever
> mount /dev/wahtever /lib/firmware
> # Put it on /etc/fstab too
> 
> Generate 1000 random files on it:
> 
> ```
> for n in {1..1000}; do                                                          
>     dd if=/dev/urandom of=/lib/firmware/file$( printf %03d "$n" ).bin bs=1 count=$((RANDOM + 1024 ))
> done  
> ```
> 
> Then reboot, upon reboot do:
> 
> modprobe test_firmware
> echo 1 > /sys/devices/virtual/misc/test_firmware/config_enable_resume_test
> systemctl suspend

OK this fixes it but this just shows that likely the thaw'ing allows
a race to take place which we didn't expect. I'll do some more digging
for a proper fix.

diff --git a/fs/kernel_read_file.c b/fs/kernel_read_file.c
index 90d255fbdd9b..b9c37eefab35 100644
--- a/fs/kernel_read_file.c
+++ b/fs/kernel_read_file.c
@@ -4,6 +4,7 @@
 #include <linux/kernel_read_file.h>
 #include <linux/security.h>
 #include <linux/vmalloc.h>
+#include <linux/umh.h>
 
 /**
  * kernel_read_file() - read file contents into a kernel buffer
@@ -156,17 +157,25 @@ int kernel_read_file_from_path_initns(const char *path, loff_t offset,
 	if (!path || !*path)
 		return -EINVAL;
 
+	ret = usermodehelper_read_trylock();
+	if (WARN_ON(ret)) {
+		pr_warn_once("Trying to do direct read when not available");
+		return ret;
+	}
 	task_lock(&init_task);
 	get_fs_root(init_task.fs, &root);
 	task_unlock(&init_task);
 
 	file = file_open_root(root.dentry, root.mnt, path, O_RDONLY, 0);
 	path_put(&root);
-	if (IS_ERR(file))
+	if (IS_ERR(file)) {
+		usermodehelper_read_unlock();
 		return PTR_ERR(file);
+	}
 
 	ret = kernel_read_file(file, offset, buf, buf_size, file_size, id);
 	fput(file);
+	usermodehelper_read_unlock();
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path_initns);