Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
To:     Luis Chamberlain <mcgrof@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
From:   Topi Miettinen <toiwoton@gmail.com>
Subject: [PATCH] firmware_loader: load files from the mount namespace of init
Message-ID: <bb46ebae-4746-90d9-ec5b-fce4c9328c86@gmail.com>
Date:   Fri, 17 Jan 2020 20:36:13 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.9.0
MIME-Version: 1.0
Content-Type: multipart/mixed;
 boundary="------------299D982EB9ACEFC63BDD3988"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

This is a multi-part message in MIME format.
--------------299D982EB9ACEFC63BDD3988
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

Hi,

I have an experimental setup where almost every possible system service 
(even early startup ones) runs in separate namespace, using a dedicated, 
minimal file system. In process of minimizing the contents of the file 
systems with regards to modules and firmware files, I noticed that in my 
system, the firmware files are loaded from three different mount 
namespaces, those of systemd-udevd, init and systemd-networkd. The logic 
of the source namespace is not very clear, it seems to depend on the 
driver, but the namespace of the current process is used.

So, this patch tries to make things a bit clearer and changes the 
loading of firmware files only from the mount namespace of init. This 
may also improve security, though I think that using firmware files as 
attack vector could be too impractical anyway.

Later, it might make sense to make the mount namespace configurable, for 
example with a new file in
/proc/sys/kernel/firmware_config/. That would allow a dedicated file 
system only for firmware files and those need not be present anywhere 
else. This configurability would make more sense if made also for kernel 
modules and /sbin/modprobe. Modules are already loaded from init 
namespace (usermodehelper uses kthreadd namespace) except when directly 
loaded by systemd-udevd.

-Topi

--------------299D982EB9ACEFC63BDD3988
Content-Type: text/x-diff;
 name="0001-firmware_loader-load-files-from-the-mount-namespace-.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
 filename*0="0001-firmware_loader-load-files-from-the-mount-namespace-.pa";
 filename*1="tch"

From 8adbf10d6b5a5e011b83fcf952e5dd4b4f2b749e Mon Sep 17 00:00:00 2001
From: Topi Miettinen <toiwoton@gmail.com>
Date: Fri, 17 Jan 2020 19:56:45 +0200
Subject: [PATCH] firmware_loader: load files from the mount namespace of init

Instead of using the mount namespace of the current process to load
firmware files, use the mount namespace of init process.

Signed-off-by: Topi Miettinen <toiwoton@gmail.com>
---
 drivers/base/firmware_loader/main.c |  6 ++++--
 fs/exec.c                           | 25 +++++++++++++++++++++++++
 include/linux/fs.h                  |  2 ++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 249add8c5e05..01f5315fae53 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -493,8 +493,10 @@ fw_get_filesystem_firmware(struct device *device, struct fw_priv *fw_priv,
 		}
 
 		fw_priv->size = 0;
-		rc = kernel_read_file_from_path(path, &buffer, &size,
-						msize, id);
+
+		/* load firmware files from the mount namespace of init */
+		rc = kernel_read_file_from_path_initns(path, &buffer,
+						       &size, msize, id);
 		if (rc) {
 			if (rc != -ENOENT)
 				dev_warn(device, "loading %s failed with error %d\n",
diff --git a/fs/exec.c b/fs/exec.c
index 74d88dab98dd..fda1364dc142 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -981,6 +981,31 @@ int kernel_read_file_from_path(const char *path, void **buf, loff_t *size,
 }
 EXPORT_SYMBOL_GPL(kernel_read_file_from_path);
 
+int kernel_read_file_from_path_initns(const char *path, void **buf,
+				      loff_t *size, loff_t max_size,
+				      enum kernel_read_file_id id)
+{
+	struct file *file;
+	struct path root;
+	int ret;
+
+	if (!path || !*path)
+		return -EINVAL;
+
+	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))
+		return PTR_ERR(file);
+
+	ret = kernel_read_file(file, buf, size, max_size, id);
+	fput(file);
+	return ret;
+}
+
 int kernel_read_file_from_fd(int fd, void **buf, loff_t *size, loff_t max_size,
 			     enum kernel_read_file_id id)
 {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 98e0349adb52..616a64871b2e 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2994,6 +2994,8 @@ extern int kernel_read_file(struct file *, void **, loff_t *, loff_t,
 			    enum kernel_read_file_id);
 extern int kernel_read_file_from_path(const char *, void **, loff_t *, loff_t,
 				      enum kernel_read_file_id);
+extern int kernel_read_file_from_path_initns(const char *, void **, loff_t *, loff_t,
+					     enum kernel_read_file_id);
 extern int kernel_read_file_from_fd(int, void **, loff_t *, loff_t,
 				    enum kernel_read_file_id);
 extern ssize_t kernel_read(struct file *, void *, size_t, loff_t *);
-- 
2.24.1


--------------299D982EB9ACEFC63BDD3988--