Re: [PATCH 00/14] Make the user mode driver code a better citizen

[Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>]

On 2020/06/27 21:59, Eric W. Biederman wrote:
> Can you try replacing the __fput_sync with:
> 	fput(file);
>         flush_delayed_fput();
>         task_work_run();

With below change, TOMOYO can obtain pathname like "tmpfs:/my\040test\040driver".

Please avoid WARN_ON() if printk() is sufficient (for friendliness to panic_on_warn=1 environments).
For argv[], I guess that fork_usermode_driver() should receive argv[] as argument rather than
trying to split info->driver_name, for somebody might want to pass meaningful argv[] (and
TOMOYO wants to use meaningful argv[] as a hint for identifying the intent).

diff --git a/kernel/umd.c b/kernel/umd.c
index de2f542191e5..ae6e85283f13 100644
--- a/kernel/umd.c
+++ b/kernel/umd.c
@@ -7,6 +7,7 @@
 #include <linux/mount.h>
 #include <linux/fs_struct.h>
 #include <linux/umd.h>
+#include <linux/task_work.h>
 
 static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *name)
 {
@@ -25,7 +26,7 @@ static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *na
 	if (IS_ERR(mnt))
 		return mnt;
 
-	file = file_open_root(mnt->mnt_root, mnt, name, O_CREAT | O_WRONLY, 0700);
+	file = file_open_root(mnt->mnt_root, mnt, name, O_CREAT | O_WRONLY | O_EXCL, 0700);
 	if (IS_ERR(file)) {
 		mntput(mnt);
 		return ERR_CAST(file);
@@ -41,23 +42,33 @@ static struct vfsmount *blob_to_mnt(const void *data, size_t len, const char *na
 		return ERR_PTR(err);
 	}
 
-	__fput_sync(file);
+	if (current->flags & PF_KTHREAD) {
+		__fput_sync(file);
+	} else {
+		fput(file);
+		flush_delayed_fput();
+		task_work_run();
+	}
 	return mnt;
 }
 
 /**
  * umd_load_blob - Remember a blob of bytes for fork_usermode_driver
- * @info: information about usermode driver
- * @data: a blob of bytes that can be executed as a file
- * @len:  The lentgh of the blob
+ * @info: information about usermode driver (shouldn't be NULL)
+ * @data: a blob of bytes that can be executed as a file (shouldn't be NULL)
+ * @len:  The lentgh of the blob (shouldn't be 0)
  *
  */
 int umd_load_blob(struct umd_info *info, const void *data, size_t len)
 {
 	struct vfsmount *mnt;
 
-	if (WARN_ON_ONCE(info->wd.dentry || info->wd.mnt))
+	if (!info || !info->driver_name || !data || !len)
+		return -EINVAL;
+	if (info->wd.dentry || info->wd.mnt) {
+		pr_info("%s already loaded.\n", info->driver_name);
 		return -EBUSY;
+	}
 
 	mnt = blob_to_mnt(data, len, info->driver_name);
 	if (IS_ERR(mnt))
@@ -71,14 +82,14 @@ EXPORT_SYMBOL_GPL(umd_load_blob);
 
 /**
  * umd_unload_blob - Disassociate @info from a previously loaded blob
- * @info: information about usermode driver
+ * @info: information about usermode driver (shouldn't be NULL)
  *
  */
 int umd_unload_blob(struct umd_info *info)
 {
-	if (WARN_ON_ONCE(!info->wd.mnt ||
-			 !info->wd.dentry ||
-			 info->wd.mnt->mnt_root != info->wd.dentry))
+	if (!info || !info->driver_name || !info->wd.dentry || !info->wd.mnt)
+		return -EINVAL;
+	if (WARN_ON_ONCE(info->wd.mnt->mnt_root != info->wd.dentry))
 		return -EINVAL;
 
 	kern_unmount(info->wd.mnt);
@@ -158,8 +169,14 @@ int fork_usermode_driver(struct umd_info *info)
 	char **argv = NULL;
 	int err;
 
-	if (WARN_ON_ONCE(info->tgid))
+	if (!info || !info->driver_name || !info->wd.dentry || !info->wd.mnt)
+		return -EINVAL;
+	if (WARN_ON_ONCE(info->wd.mnt->mnt_root != info->wd.dentry))
+		return -EINVAL;
+	if (info->tgid) {
+		pr_info("%s already running.\n", info->driver_name);
 		return -EBUSY;
+	}
 
 	err = -ENOMEM;
 	argv = argv_split(GFP_KERNEL, info->driver_name, NULL);