[PATCH 0/5] usermode_driver: Add management library and API

[PATCH 0/5] usermode_driver: Add management library and API

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
which runs a user space process from a binary blob, and creates a
bidirectional pipe, so that the kernel can make a request to that process,
and the latter provides its response. It is currently used by bpfilter,
although it does not seem to do any useful work.

The problem is, if other users would like to implement a UMD similar to
bpfilter, they would have to duplicate the code. Instead, make an UMD
management library and API from the existing bpfilter and sockopt code,
and move it to common kernel code.

Also, define the software architecture and the main components of the
library: the UMD Manager, running in the kernel, acting as the frontend
interface to any user or kernel-originated request; the UMD Loader, also
running in the kernel, responsible to load the UMD Handler; the UMD
Handler, running in user space, responsible to handle requests from the UMD
Manager and to send to it the response.

I have two use cases, but for sake of brevity I will propose one.

I would like to add support for PGP keys and signatures in the kernel, so
that I can extend secure boot to applications, and allow/deny code
execution based on the signed file digests included in RPM headers.

While I proposed a patch set a while ago (based on a previous work of David
Howells), the main objection was that the PGP packet parser should not run
in the kernel.

That makes a perfect example for using a UMD. If the PGP parser is moved to
user space (UMD Handler), and the kernel (UMD Manager) just instantiates
the key and verifies the signature on already parsed data, this would
address the concern.

Patch 1 moves the function bpfilter_send_req() to usermode_driver.c and
makes the pipe between the kernel and the user space process suitable for
larger quantity of data (> 64K).

Patch 2 introduces the management library and API.

Patch 3 replaces the existing bpfilter and sockopt code with calls
to the management API. To use the new mechanism, sockopt itself (acts as
UMD Manager) now sends/receives messages to/from bpfilter_umh (acts as UMD
Handler), instead of bpfilter (acts as UMD Loader).

Patch 4 introduces a sample UMD, useful for other implementors, and uses it
for testing.

Patch 5 introduces the documentation of the new management library and API.

Roberto Sassu (5):
  usermode_driver: Introduce umd_send_recv() from bpfilter
  usermode_driver_mgmt: Introduce management of user mode drivers
  bpfilter: Port to user mode driver management API
  selftests/umd_mgmt: Add selftests for UMD management library
  doc: Add documentation for the User Mode Driver management library

 Documentation/driver-api/index.rst            |   1 +
 Documentation/driver-api/umd_mgmt.rst         |  99 +++++++++++++
 MAINTAINERS                                   |   9 ++
 include/linux/bpfilter.h                      |  12 +-
 include/linux/usermode_driver.h               |   2 +
 include/linux/usermode_driver_mgmt.h          |  35 +++++
 kernel/Makefile                               |   2 +-
 kernel/usermode_driver.c                      |  47 +++++-
 kernel/usermode_driver_mgmt.c                 | 137 ++++++++++++++++++
 net/bpfilter/bpfilter_kern.c                  | 120 +--------------
 net/ipv4/bpfilter/sockopt.c                   |  67 +++++----
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/umd_mgmt/.gitignore   |   1 +
 tools/testing/selftests/umd_mgmt/Makefile     |  14 ++
 tools/testing/selftests/umd_mgmt/config       |   1 +
 .../selftests/umd_mgmt/sample_umd/Makefile    |  22 +++
 .../selftests/umd_mgmt/sample_umd/msgfmt.h    |  13 ++
 .../umd_mgmt/sample_umd/sample_binary_blob.S  |   7 +
 .../umd_mgmt/sample_umd/sample_handler.c      |  81 +++++++++++
 .../umd_mgmt/sample_umd/sample_loader.c       |  28 ++++
 .../umd_mgmt/sample_umd/sample_mgr.c          | 124 ++++++++++++++++
 tools/testing/selftests/umd_mgmt/umd_mgmt.sh  |  40 +++++
 22 files changed, 707 insertions(+), 156 deletions(-)
 create mode 100644 Documentation/driver-api/umd_mgmt.rst
 create mode 100644 include/linux/usermode_driver_mgmt.h
 create mode 100644 kernel/usermode_driver_mgmt.c
 create mode 100644 tools/testing/selftests/umd_mgmt/.gitignore
 create mode 100644 tools/testing/selftests/umd_mgmt/Makefile
 create mode 100644 tools/testing/selftests/umd_mgmt/config
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/Makefile
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c
 create mode 100755 tools/testing/selftests/umd_mgmt/umd_mgmt.sh

-- 
2.25.1

[PATCH 1/5] usermode_driver: Introduce umd_send_recv() from bpfilter

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Move bpfilter_send_req() to usermode_driver.c and rename it to
umd_send_recv(). Make the latter independent from the message format by
passing to it the pointers to the request and reply message buffers and the
respective lengths, and by merely doing read/write operations.

From umd_send_recv(), remove the call to __stop_umh() and returning
reply.status, which is message format-specific. Let the callers of
umd_send_recv(), such as bpfilter_process_sockopt(), call shutdown_umh()
and evaluate the reply. Consequently, remove __stop_umh(), since in
bpfilter_process_sockopt() the CONFIG_INET condition is always true.

In addition to the original bpfilter_send_req() implementation, support
partial reads and writes, so that callers are not limited by the length of
the message to send or receive. Currently, the pipe supports receiving
64 KB at once.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/usermode_driver.h |  2 ++
 kernel/usermode_driver.c        | 47 ++++++++++++++++++++++++++++++-
 net/bpfilter/bpfilter_kern.c    | 50 +++++++++------------------------
 3 files changed, 62 insertions(+), 37 deletions(-)

diff --git a/include/linux/usermode_driver.h b/include/linux/usermode_driver.h
index ad970416260..37b8d08cc3d 100644
--- a/include/linux/usermode_driver.h
+++ b/include/linux/usermode_driver.h
@@ -15,5 +15,7 @@ int umd_load_blob(struct umd_info *info, const void *data, size_t len);
 int umd_unload_blob(struct umd_info *info);
 int fork_usermode_driver(struct umd_info *info);
 void umd_cleanup_helper(struct umd_info *info);
+int umd_send_recv(struct umd_info *info, void *in, size_t in_len, void *out,
+		  size_t out_len);
 
 #endif /* __LINUX_USERMODE_DRIVER_H__ */
diff --git a/kernel/usermode_driver.c b/kernel/usermode_driver.c
index 8303f4c7ca7..cdbfaad99d7 100644
--- a/kernel/usermode_driver.c
+++ b/kernel/usermode_driver.c
@@ -188,4 +188,49 @@ int fork_usermode_driver(struct umd_info *info)
 }
 EXPORT_SYMBOL_GPL(fork_usermode_driver);
 
-
+/**
+ * umd_send_recv - send/receive a message through the pipe
+ * @info: user mode driver info
+ * @in: request message
+ * @in_len: size of @in
+ * @out: reply message
+ * @out_len: size of @out
+ *
+ * Send a message to the user space process through the created pipe and read
+ * the reply. Partial reads and writes are supported.
+ *
+ * Return: Zero on success, -EFAULT otherwise.
+ */
+int umd_send_recv(struct umd_info *info, void *in, size_t in_len, void *out,
+		  size_t out_len)
+{
+	loff_t pos, offset;
+	ssize_t n;
+
+	if (!info->tgid)
+		return -EFAULT;
+	pos = 0;
+	offset = 0;
+	while (in_len) {
+		n = kernel_write(info->pipe_to_umh, in + offset, in_len, &pos);
+		if (n <= 0) {
+			pr_err("write fail %zd\n", n);
+			return -EFAULT;
+		}
+		in_len -= n;
+		offset += n;
+	}
+	pos = 0;
+	offset = 0;
+	while (out_len) {
+		n = kernel_read(info->pipe_from_umh, out + offset, out_len, &pos);
+		if (n <= 0) {
+			pr_err("read fail %zd\n", n);
+			return -EFAULT;
+		}
+		out_len -= n;
+		offset += n;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(umd_send_recv);
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 422ec6e7ccf..17d4df5f8fe 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -25,40 +25,6 @@ static void shutdown_umh(void)
 	}
 }
 
-static void __stop_umh(void)
-{
-	if (IS_ENABLED(CONFIG_INET))
-		shutdown_umh();
-}
-
-static int bpfilter_send_req(struct mbox_request *req)
-{
-	struct mbox_reply reply;
-	loff_t pos = 0;
-	ssize_t n;
-
-	if (!bpfilter_ops.info.tgid)
-		return -EFAULT;
-	pos = 0;
-	n = kernel_write(bpfilter_ops.info.pipe_to_umh, req, sizeof(*req),
-			   &pos);
-	if (n != sizeof(*req)) {
-		pr_err("write fail %zd\n", n);
-		goto stop;
-	}
-	pos = 0;
-	n = kernel_read(bpfilter_ops.info.pipe_from_umh, &reply, sizeof(reply),
-			&pos);
-	if (n != sizeof(reply)) {
-		pr_err("read fail %zd\n", n);
-		goto stop;
-	}
-	return reply.status;
-stop:
-	__stop_umh();
-	return -EFAULT;
-}
-
 static int bpfilter_process_sockopt(struct sock *sk, int optname,
 				    sockptr_t optval, unsigned int optlen,
 				    bool is_set)
@@ -70,16 +36,27 @@ static int bpfilter_process_sockopt(struct sock *sk, int optname,
 		.addr		= (uintptr_t)optval.user,
 		.len		= optlen,
 	};
+	struct mbox_reply reply;
+	int err;
+
 	if (sockptr_is_kernel(optval)) {
 		pr_err("kernel access not supported\n");
 		return -EFAULT;
 	}
-	return bpfilter_send_req(&req);
+	err = umd_send_recv(&bpfilter_ops.info, &req, sizeof(req), &reply,
+			    sizeof(reply));
+	if (err) {
+		shutdown_umh();
+		return err;
+	}
+
+	return reply.status;
 }
 
 static int start_umh(void)
 {
 	struct mbox_request req = { .pid = current->pid };
+	struct mbox_reply reply;
 	int err;
 
 	/* fork usermode process */
@@ -89,7 +66,8 @@ static int start_umh(void)
 	pr_info("Loaded bpfilter_umh pid %d\n", pid_nr(bpfilter_ops.info.tgid));
 
 	/* health check that usermode process started correctly */
-	if (bpfilter_send_req(&req) != 0) {
+	if (umd_send_recv(&bpfilter_ops.info, &req, sizeof(req), &reply,
+			  sizeof(reply)) != 0 || reply.status != 0) {
 		shutdown_umh();
 		return -EFAULT;
 	}
-- 
2.25.1

[PATCH 2/5] usermode_driver_mgmt: Introduce management of user mode drivers

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Reuse the bpfilter code, with some adjustments to make the code more
generic, and usable for other user mode drivers.

:: struct umd_mgmt <=== struct bpfilter_umh_ops ::
Replace the start method with post_start, to allow for some customization
of the start procedure. All start procedures have in common the call to
fork_usermode_driver().

Remove the sockopt method, as it is use case-specific. Instead, use one of
the following two alternatives: generate the message from the manager of
the driver (e.g. sockopt), by exporting the message format definition; or,
define a new structure embedding the umd_mgmt and the custom method, which
can be registered from the kernel module through the post_start method.

Add kmod and kmod_loaded. Kmod is name of the kernel module that
umd_mgmt_send_recv() (from bpfilter_mbox_request()) attempts to load if
kmod_loaded is false (the driver is not yet started). Kmod_loaded is added
to replace the sockopt method test, and ensure that the driver is ready for
use.

:: start_umh() <=== start_umh() ::
Remove the connection test, and call the post_start method in umd_mgmt, if
set, which could point to that test.

:: shutdown_umh() <=== shutdown_umh() ::
Same code.

:: umd_mgmt_send_recv() <=== bpfilter_mbox_request() ::
Replace the bpfilter_mbox_request() parameters with the parameters of
umd_send_recv(), except for the first which is a umd_mgmt structure instead
of umd_info. Also, call umd_send_recv() instead of the sockopt method, and
shutdown the driver if there is an error.

:: umd_mgmt_load() <=== load_umh() ::
Same code except for the registration of the start and sockopt methods,
replaced with setting kmod_loaded to true (not depending on CONFIG_INET),
as mentioned in the explanation of umd_mgmt.

:: umd_mgmt_unload() <=== fini_umh() ::
Same code except for the deregistration of the start and sockopt methods,
replaced with setting kmod_loaded to false (not depending on CONFIG_INET).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 MAINTAINERS                          |   7 ++
 include/linux/usermode_driver_mgmt.h |  35 +++++++
 kernel/Makefile                      |   2 +-
 kernel/usermode_driver_mgmt.c        | 137 +++++++++++++++++++++++++++
 4 files changed, 180 insertions(+), 1 deletion(-)
 create mode 100644 include/linux/usermode_driver_mgmt.h
 create mode 100644 kernel/usermode_driver_mgmt.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a3b14ec3383..7b27435fd20 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11245,6 +11245,13 @@ S:	Maintained
 F:	include/linux/umh.h
 F:	kernel/umh.c
 
+KERNEL USERMODE DRIVER MANAGEMENT
+M:	Roberto Sassu <roberto.sassu@huawei.com>
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	include/linux/usermode_driver_mgmt.h
+F:	kernel/usermode_driver_mgmt.c
+
 KERNEL VIRTUAL MACHINE (KVM)
 M:	Paolo Bonzini <pbonzini@redhat.com>
 L:	kvm@vger.kernel.org
diff --git a/include/linux/usermode_driver_mgmt.h b/include/linux/usermode_driver_mgmt.h
new file mode 100644
index 00000000000..3f9fc783a09
--- /dev/null
+++ b/include/linux/usermode_driver_mgmt.h
@@ -0,0 +1,35 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
+ *
+ * User mode driver management API.
+ */
+#ifndef __LINUX_USERMODE_DRIVER_MGMT_H__
+#define __LINUX_USERMODE_DRIVER_MGMT_H__
+
+#include <linux/usermode_driver.h>
+
+/**
+ * struct umd_mgmt - user mode driver management structure
+ * @info: user mode driver information
+ * @lock: lock to serialize requests to the UMD Handler
+ * @post_start: function with additional operations after UMD Handler is started
+ * @kmod: kernel module acting as UMD Loader, to start the UMD Handler
+ * @kmod_loaded: whether @kmod is loaded or not
+ *
+ * Information necessary to manage the UMD during its lifecycle.
+ */
+struct umd_mgmt {
+	struct umd_info info;
+	struct mutex lock;
+	int (*post_start)(struct umd_mgmt *mgmt);
+	const char *kmod;
+	bool kmod_loaded;
+};
+
+int umd_mgmt_send_recv(struct umd_mgmt *mgmt, void *in, size_t in_len,
+		       void *out, size_t out_len);
+int umd_mgmt_load(struct umd_mgmt *mgmt, char *start, char *end);
+void umd_mgmt_unload(struct umd_mgmt *mgmt);
+
+#endif /* __LINUX_USERMODE_DRIVER_MGMT_H__ */
diff --git a/kernel/Makefile b/kernel/Makefile
index 10ef068f598..ee47f7c2023 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -12,7 +12,7 @@ obj-y     = fork.o exec_domain.o panic.o \
 	    notifier.o ksysfs.o cred.o reboot.o \
 	    async.o range.o smpboot.o ucount.o regset.o
 
-obj-$(CONFIG_USERMODE_DRIVER) += usermode_driver.o
+obj-$(CONFIG_USERMODE_DRIVER) += usermode_driver.o usermode_driver_mgmt.o
 obj-$(CONFIG_MODULES) += kmod.o
 obj-$(CONFIG_MULTIUSER) += groups.o
 
diff --git a/kernel/usermode_driver_mgmt.c b/kernel/usermode_driver_mgmt.c
new file mode 100644
index 00000000000..4fb06b37f62
--- /dev/null
+++ b/kernel/usermode_driver_mgmt.c
@@ -0,0 +1,137 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
+ *
+ * User mode driver management library.
+ */
+#include <linux/kmod.h>
+#include <linux/fs.h>
+#include <linux/usermode_driver_mgmt.h>
+
+static void shutdown_umh(struct umd_mgmt *mgmt)
+{
+	struct umd_info *info = &mgmt->info;
+	struct pid *tgid = info->tgid;
+
+	if (tgid) {
+		kill_pid(tgid, SIGKILL, 1);
+		wait_event(tgid->wait_pidfd, thread_group_exited(tgid));
+		umd_cleanup_helper(info);
+	}
+}
+
+static int start_umh(struct umd_mgmt *mgmt)
+{
+	int err;
+
+	/* fork usermode process */
+	err = fork_usermode_driver(&mgmt->info);
+	if (err)
+		return err;
+	pr_info("Loaded %s pid %d\n", mgmt->info.driver_name,
+		pid_nr(mgmt->info.tgid));
+
+	if (mgmt->post_start) {
+		err = mgmt->post_start(mgmt);
+		if (err)
+			shutdown_umh(mgmt);
+	}
+
+	return err;
+}
+
+/**
+ * umd_mgmt_send_recv - Communicate with the UMD Handler and start it.
+ * @mgmt: user mode driver management structure
+ * @in: request message
+ * @in_len: size of @in
+ * @out: reply message
+ * @out_len: size of @out
+ *
+ * Send a message to the UMD Handler through the created pipe and read the
+ * reply. If the UMD Handler is not available, invoke the UMD Loader to
+ * instantiate it. If the UMD Handler exited, restart it.
+ *
+ * Return: Zero on success, a negative value otherwise.
+ */
+int umd_mgmt_send_recv(struct umd_mgmt *mgmt, void *in, size_t in_len,
+		       void *out, size_t out_len)
+{
+	int err;
+
+	mutex_lock(&mgmt->lock);
+	if (!mgmt->kmod_loaded) {
+		mutex_unlock(&mgmt->lock);
+		request_module(mgmt->kmod);
+		mutex_lock(&mgmt->lock);
+
+		if (!mgmt->kmod_loaded) {
+			err = -ENOPROTOOPT;
+			goto out;
+		}
+	}
+	if (mgmt->info.tgid &&
+	    thread_group_exited(mgmt->info.tgid))
+		umd_cleanup_helper(&mgmt->info);
+
+	if (!mgmt->info.tgid) {
+		err = start_umh(mgmt);
+		if (err)
+			goto out;
+	}
+	err = umd_send_recv(&mgmt->info, in, in_len, out, out_len);
+	if (err)
+		shutdown_umh(mgmt);
+out:
+	mutex_unlock(&mgmt->lock);
+	return err;
+}
+EXPORT_SYMBOL_GPL(umd_mgmt_send_recv);
+
+/**
+ * umd_mgmt_load - Load and start the UMD Handler.
+ * @mgmt: user mode driver management structure
+ * @start: start address of the binary blob of the UMD Handler
+ * @end: end address of the binary blob of the UMD Handler
+ *
+ * Copy the UMD Handler binary from the specified location to a private tmpfs
+ * filesystem. Then, start the UMD Handler.
+ *
+ * Return: Zero on success, a negative value otherwise.
+ */
+int umd_mgmt_load(struct umd_mgmt *mgmt, char *start, char *end)
+{
+	int err;
+
+	err = umd_load_blob(&mgmt->info, start, end - start);
+	if (err)
+		return err;
+
+	mutex_lock(&mgmt->lock);
+	err = start_umh(mgmt);
+	if (!err)
+		mgmt->kmod_loaded = true;
+	mutex_unlock(&mgmt->lock);
+	if (err)
+		umd_unload_blob(&mgmt->info);
+	return err;
+}
+EXPORT_SYMBOL_GPL(umd_mgmt_load);
+
+/**
+ * umd_mgmt_unload - Terminate and unload the UMD Handler.
+ * @mgmt: user mode driver management structure
+ *
+ * Terminate the UMD Handler, and cleanup the private tmpfs filesystem with the
+ * UMD Handler binary.
+ */
+void umd_mgmt_unload(struct umd_mgmt *mgmt)
+{
+	mutex_lock(&mgmt->lock);
+	shutdown_umh(mgmt);
+	mgmt->kmod_loaded = false;
+	mutex_unlock(&mgmt->lock);
+
+	umd_unload_blob(&mgmt->info);
+}
+EXPORT_SYMBOL_GPL(umd_mgmt_unload);
-- 
2.25.1

[PATCH 3/5] bpfilter: Port to user mode driver management API

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Move bpfilter_process_sockopt() to sockopt.c, and call umd_mgmt_send_recv()
instead of umd_send_recv(), so that it has the original capability of
bpfilter_mbox_request() to request the loading of the kernel module
containing the user mode driver.

Move also the connection test originally in start_umh() in bpfilter.c, to
sockopt.c. Create the new function bpfilter_post_start_umh() with the moved
code.

Directly call bpfilter_process_sockopt() from bpfilter_ip_set_sockopt() and
bpfilter_ip_get_sockopt(), which now it is equivalent to
bpfilter_mbox_request().

Remove the struct bpfilter_umh_ops definition, and declare the global
variable bpfilter_ops as a umd_mgmt structure.

Set kmod to 'bpfilter', kmod_loaded to false and the new function
bpfilter_post_start_umh() as the post_start method for bpfilter_ops.

Replace load_umh() and fini_umh() in bpfilter.c respectively with
umd_mgmt_load() and umd_mgmt_unload().

Finally, remove the remaining functions, as their job is done by the user
mode driver management.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/bpfilter.h     | 12 +----
 net/bpfilter/bpfilter_kern.c | 98 ++----------------------------------
 net/ipv4/bpfilter/sockopt.c  | 67 ++++++++++++++----------
 3 files changed, 45 insertions(+), 132 deletions(-)

diff --git a/include/linux/bpfilter.h b/include/linux/bpfilter.h
index 2ae3c8e1d83..655e6ec6e9d 100644
--- a/include/linux/bpfilter.h
+++ b/include/linux/bpfilter.h
@@ -3,7 +3,7 @@
 #define _LINUX_BPFILTER_H
 
 #include <uapi/linux/bpfilter.h>
-#include <linux/usermode_driver.h>
+#include <linux/usermode_driver_mgmt.h>
 #include <linux/sockptr.h>
 
 struct sock;
@@ -13,13 +13,5 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
 			    int __user *optlen);
 void bpfilter_umh_cleanup(struct umd_info *info);
 
-struct bpfilter_umh_ops {
-	struct umd_info info;
-	/* since ip_getsockopt() can run in parallel, serialize access to umh */
-	struct mutex lock;
-	int (*sockopt)(struct sock *sk, int optname, sockptr_t optval,
-		       unsigned int optlen, bool is_set);
-	int (*start)(void);
-};
-extern struct bpfilter_umh_ops bpfilter_ops;
+extern struct umd_mgmt bpfilter_ops;
 #endif
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 17d4df5f8fe..f2137d889c9 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -1,113 +1,21 @@
 // SPDX-License-Identifier: GPL-2.0
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include <linux/init.h>
 #include <linux/module.h>
-#include <linux/umh.h>
 #include <linux/bpfilter.h>
-#include <linux/sched.h>
-#include <linux/sched/signal.h>
-#include <linux/fs.h>
-#include <linux/file.h>
 #include "msgfmt.h"
 
 extern char bpfilter_umh_start;
 extern char bpfilter_umh_end;
 
-static void shutdown_umh(void)
-{
-	struct umd_info *info = &bpfilter_ops.info;
-	struct pid *tgid = info->tgid;
-
-	if (tgid) {
-		kill_pid(tgid, SIGKILL, 1);
-		wait_event(tgid->wait_pidfd, thread_group_exited(tgid));
-		bpfilter_umh_cleanup(info);
-	}
-}
-
-static int bpfilter_process_sockopt(struct sock *sk, int optname,
-				    sockptr_t optval, unsigned int optlen,
-				    bool is_set)
-{
-	struct mbox_request req = {
-		.is_set		= is_set,
-		.pid		= current->pid,
-		.cmd		= optname,
-		.addr		= (uintptr_t)optval.user,
-		.len		= optlen,
-	};
-	struct mbox_reply reply;
-	int err;
-
-	if (sockptr_is_kernel(optval)) {
-		pr_err("kernel access not supported\n");
-		return -EFAULT;
-	}
-	err = umd_send_recv(&bpfilter_ops.info, &req, sizeof(req), &reply,
-			    sizeof(reply));
-	if (err) {
-		shutdown_umh();
-		return err;
-	}
-
-	return reply.status;
-}
-
-static int start_umh(void)
-{
-	struct mbox_request req = { .pid = current->pid };
-	struct mbox_reply reply;
-	int err;
-
-	/* fork usermode process */
-	err = fork_usermode_driver(&bpfilter_ops.info);
-	if (err)
-		return err;
-	pr_info("Loaded bpfilter_umh pid %d\n", pid_nr(bpfilter_ops.info.tgid));
-
-	/* health check that usermode process started correctly */
-	if (umd_send_recv(&bpfilter_ops.info, &req, sizeof(req), &reply,
-			  sizeof(reply)) != 0 || reply.status != 0) {
-		shutdown_umh();
-		return -EFAULT;
-	}
-
-	return 0;
-}
-
 static int __init load_umh(void)
 {
-	int err;
-
-	err = umd_load_blob(&bpfilter_ops.info,
-			    &bpfilter_umh_start,
-			    &bpfilter_umh_end - &bpfilter_umh_start);
-	if (err)
-		return err;
-
-	mutex_lock(&bpfilter_ops.lock);
-	err = start_umh();
-	if (!err && IS_ENABLED(CONFIG_INET)) {
-		bpfilter_ops.sockopt = &bpfilter_process_sockopt;
-		bpfilter_ops.start = &start_umh;
-	}
-	mutex_unlock(&bpfilter_ops.lock);
-	if (err)
-		umd_unload_blob(&bpfilter_ops.info);
-	return err;
+	return umd_mgmt_load(&bpfilter_ops, &bpfilter_umh_start,
+			     &bpfilter_umh_end);
 }
 
 static void __exit fini_umh(void)
 {
-	mutex_lock(&bpfilter_ops.lock);
-	if (IS_ENABLED(CONFIG_INET)) {
-		shutdown_umh();
-		bpfilter_ops.start = NULL;
-		bpfilter_ops.sockopt = NULL;
-	}
-	mutex_unlock(&bpfilter_ops.lock);
-
-	umd_unload_blob(&bpfilter_ops.info);
+	umd_mgmt_unload(&bpfilter_ops);
 }
 module_init(load_umh);
 module_exit(fini_umh);
diff --git a/net/ipv4/bpfilter/sockopt.c b/net/ipv4/bpfilter/sockopt.c
index 1b34cb9a770..491f5042612 100644
--- a/net/ipv4/bpfilter/sockopt.c
+++ b/net/ipv4/bpfilter/sockopt.c
@@ -8,8 +8,9 @@
 #include <linux/kmod.h>
 #include <linux/fs.h>
 #include <linux/file.h>
+#include "../../bpfilter/msgfmt.h"
 
-struct bpfilter_umh_ops bpfilter_ops;
+struct umd_mgmt bpfilter_ops;
 EXPORT_SYMBOL_GPL(bpfilter_ops);
 
 void bpfilter_umh_cleanup(struct umd_info *info)
@@ -21,40 +22,49 @@ void bpfilter_umh_cleanup(struct umd_info *info)
 }
 EXPORT_SYMBOL_GPL(bpfilter_umh_cleanup);
 
-static int bpfilter_mbox_request(struct sock *sk, int optname, sockptr_t optval,
-				 unsigned int optlen, bool is_set)
+static int bpfilter_process_sockopt(struct sock *sk, int optname,
+				    sockptr_t optval, unsigned int optlen,
+				    bool is_set)
 {
+	struct mbox_request req = {
+		.is_set		= is_set,
+		.pid		= current->pid,
+		.cmd		= optname,
+		.addr		= (uintptr_t)optval.user,
+		.len		= optlen,
+	};
+	struct mbox_reply reply;
 	int err;
-	mutex_lock(&bpfilter_ops.lock);
-	if (!bpfilter_ops.sockopt) {
-		mutex_unlock(&bpfilter_ops.lock);
-		request_module("bpfilter");
-		mutex_lock(&bpfilter_ops.lock);
 
-		if (!bpfilter_ops.sockopt) {
-			err = -ENOPROTOOPT;
-			goto out;
-		}
+	if (sockptr_is_kernel(optval)) {
+		pr_err("kernel access not supported\n");
+		return -EFAULT;
 	}
-	if (bpfilter_ops.info.tgid &&
-	    thread_group_exited(bpfilter_ops.info.tgid))
-		bpfilter_umh_cleanup(&bpfilter_ops.info);
+	err = umd_mgmt_send_recv(&bpfilter_ops, &req, sizeof(req), &reply,
+				 sizeof(reply));
+	if (err)
+		return err;
 
-	if (!bpfilter_ops.info.tgid) {
-		err = bpfilter_ops.start();
-		if (err)
-			goto out;
-	}
-	err = bpfilter_ops.sockopt(sk, optname, optval, optlen, is_set);
-out:
-	mutex_unlock(&bpfilter_ops.lock);
-	return err;
+	return reply.status;
+}
+
+static int bpfilter_post_start_umh(struct umd_mgmt *mgmt)
+{
+	struct mbox_request req = { .pid = current->pid };
+	struct mbox_reply reply;
+
+	/* health check that usermode process started correctly */
+	if (umd_send_recv(&bpfilter_ops.info, &req, sizeof(req), &reply,
+			  sizeof(reply)) != 0 || reply.status != 0)
+		return -EFAULT;
+
+	return 0;
 }
 
 int bpfilter_ip_set_sockopt(struct sock *sk, int optname, sockptr_t optval,
 			    unsigned int optlen)
 {
-	return bpfilter_mbox_request(sk, optname, optval, optlen, true);
+	return bpfilter_process_sockopt(sk, optname, optval, optlen, true);
 }
 
 int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
@@ -65,8 +75,8 @@ int bpfilter_ip_get_sockopt(struct sock *sk, int optname, char __user *optval,
 	if (get_user(len, optlen))
 		return -EFAULT;
 
-	return bpfilter_mbox_request(sk, optname, USER_SOCKPTR(optval), len,
-				     false);
+	return bpfilter_process_sockopt(sk, optname, USER_SOCKPTR(optval), len,
+					false);
 }
 
 static int __init bpfilter_sockopt_init(void)
@@ -74,6 +84,9 @@ static int __init bpfilter_sockopt_init(void)
 	mutex_init(&bpfilter_ops.lock);
 	bpfilter_ops.info.tgid = NULL;
 	bpfilter_ops.info.driver_name = "bpfilter_umh";
+	bpfilter_ops.post_start = bpfilter_post_start_umh;
+	bpfilter_ops.kmod = "bpfilter";
+	bpfilter_ops.kmod_loaded = false;
 
 	return 0;
 }
-- 
2.25.1

[PATCH 4/5] selftests/umd_mgmt: Add selftests for UMD management library

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce a simple UMD driver using the management library, for testing
purposes.

UMD Manager: sample_mgr.c
UMD Loader: sample_loader.c
UMD Handler: sample_handler.c

The UMD Manager exposes /sys/kernel/security/sample_umd and accepts an
offset between 0-128K. It invokes the UMD Loader to start the UMD Handler
and passes to the latter the offset at which it sets the byte of the
response buffer to 1. The UMD Manager verifies that and returns the number
of bytes written on success, a negative value on failure.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 MAINTAINERS                                   |   1 +
 tools/testing/selftests/Makefile              |   1 +
 tools/testing/selftests/umd_mgmt/.gitignore   |   1 +
 tools/testing/selftests/umd_mgmt/Makefile     |  14 ++
 tools/testing/selftests/umd_mgmt/config       |   1 +
 .../selftests/umd_mgmt/sample_umd/Makefile    |  22 ++++
 .../selftests/umd_mgmt/sample_umd/msgfmt.h    |  13 ++
 .../umd_mgmt/sample_umd/sample_binary_blob.S  |   7 +
 .../umd_mgmt/sample_umd/sample_handler.c      |  81 ++++++++++++
 .../umd_mgmt/sample_umd/sample_loader.c       |  28 ++++
 .../umd_mgmt/sample_umd/sample_mgr.c          | 124 ++++++++++++++++++
 tools/testing/selftests/umd_mgmt/umd_mgmt.sh  |  40 ++++++
 12 files changed, 333 insertions(+)
 create mode 100644 tools/testing/selftests/umd_mgmt/.gitignore
 create mode 100644 tools/testing/selftests/umd_mgmt/Makefile
 create mode 100644 tools/testing/selftests/umd_mgmt/config
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/Makefile
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c
 create mode 100644 tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c
 create mode 100755 tools/testing/selftests/umd_mgmt/umd_mgmt.sh

diff --git a/MAINTAINERS b/MAINTAINERS
index 7b27435fd20..a0cd161843e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11251,6 +11251,7 @@ L:	linux-kernel@vger.kernel.org
 S:	Maintained
 F:	include/linux/usermode_driver_mgmt.h
 F:	kernel/usermode_driver_mgmt.c
+F:	tools/testing/selftests/umd_mgmt/*
 
 KERNEL VIRTUAL MACHINE (KVM)
 M:	Paolo Bonzini <pbonzini@redhat.com>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 13a6837a0c6..84202d5b4fb 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -84,6 +84,7 @@ TARGETS += timers
 endif
 TARGETS += tmpfs
 TARGETS += tpm2
+TARGETS += umd_mgmt
 TARGETS += user
 TARGETS += vDSO
 TARGETS += mm
diff --git a/tools/testing/selftests/umd_mgmt/.gitignore b/tools/testing/selftests/umd_mgmt/.gitignore
new file mode 100644
index 00000000000..215c17d13e9
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/.gitignore
@@ -0,0 +1 @@
+/sample_umd/sample_umh
diff --git a/tools/testing/selftests/umd_mgmt/Makefile b/tools/testing/selftests/umd_mgmt/Makefile
new file mode 100644
index 00000000000..f1d47eec04e
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/Makefile
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0
+TEST_GEN_PROGS_EXTENDED = sample_umd.ko
+
+$(OUTPUT)/%.ko: $(wildcard sample_umd/Makefile sample_umd/*.[ch])
+	$(call msg,MOD,,$@)
+	$(Q)$(MAKE) -C sample_umd install
+
+OVERRIDE_TARGETS := 1
+override define CLEAN
+	$(call msg,CLEAN)
+	$(Q)$(MAKE) -C sample_umd clean
+endef
+
+include ../lib.mk
diff --git a/tools/testing/selftests/umd_mgmt/config b/tools/testing/selftests/umd_mgmt/config
new file mode 100644
index 00000000000..71a078a3ac0
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/config
@@ -0,0 +1 @@
+CONFIG_BPFILTER=y
diff --git a/tools/testing/selftests/umd_mgmt/sample_umd/Makefile b/tools/testing/selftests/umd_mgmt/sample_umd/Makefile
new file mode 100644
index 00000000000..6d950e05f3d
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/sample_umd/Makefile
@@ -0,0 +1,22 @@
+KDIR ?= ../../../../..
+
+userprogs := sample_umh
+sample_umh-objs := sample_handler.o
+userccflags += -I $(srctree)
+
+$(obj)/sample_binary_blob.o: $(obj)/sample_umh
+
+MODULES = sample_loader_kmod.ko sample_mgr.ko
+
+obj-m += sample_loader_kmod.o sample_mgr.o
+
+sample_loader_kmod-objs = sample_loader.o sample_binary_blob.o
+
+all:
+	+$(Q)$(MAKE) -C $(KDIR) M=$$PWD modules
+
+clean:
+	+$(Q)$(MAKE) -C $(KDIR) M=$$PWD clean
+
+install: all
+	+$(Q)$(MAKE) -C $(KDIR) M=$$PWD modules_install
diff --git a/tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h b/tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h
new file mode 100644
index 00000000000..34a62d72cde
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/sample_umd/msgfmt.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SAMPLE_UMH_MSGFMT_H
+#define _SAMPLE_UMH_MSGFMT_H
+
+struct sample_request {
+	uint32_t offset;
+};
+
+struct sample_reply {
+	uint8_t data[128 * 1024];
+};
+
+#endif
diff --git a/tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S b/tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S
new file mode 100644
index 00000000000..3687dd13973
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/sample_umd/sample_binary_blob.S
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+	.section .init.rodata, "a"
+	.global sample_umh_start
+sample_umh_start:
+	.incbin "tools/testing/selftests/umd_mgmt/sample_umd/sample_umh"
+	.global sample_umh_end
+sample_umh_end:
diff --git a/tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c b/tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c
new file mode 100644
index 00000000000..94ea6d99bbc
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/sample_umd/sample_handler.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the UMD Handler.
+ */
+#include <unistd.h>
+#include <malloc.h>
+#include <stdint.h>
+
+#include "msgfmt.h"
+
+FILE *debug_f;
+
+static void loop(void)
+{
+	struct sample_request *req = NULL;
+	struct sample_reply *reply = NULL;
+
+	req = calloc(1, sizeof(*req));
+	if (!req)
+		return;
+
+	reply = calloc(1, sizeof(*reply));
+	if (!reply)
+		goto out;
+
+	while (1) {
+		int n, len, offset;
+
+		offset = 0;
+		len = sizeof(*req);
+
+		while (len) {
+			n = read(0, ((void *)req) + offset, len);
+			if (n <= 0) {
+				fprintf(debug_f, "invalid request %d\n", n);
+				goto out;
+			}
+
+			len -= n;
+			offset += n;
+		}
+
+		if (req->offset < sizeof(reply->data))
+			reply->data[req->offset] = 1;
+
+		offset = 0;
+		len = sizeof(*reply);
+
+		while (len) {
+			n = write(1, ((void *)reply) + offset, len);
+			if (n <= 0) {
+				fprintf(debug_f, "reply failed %d\n", n);
+				goto out;
+			}
+
+			len -= n;
+			offset += n;
+		}
+
+		if (req->offset < sizeof(reply->data))
+			reply->data[req->offset] = 0;
+	}
+out:
+	free(req);
+	free(reply);
+}
+
+int main(void)
+{
+	debug_f = fopen("/dev/kmsg", "w");
+	setvbuf(debug_f, 0, _IOLBF, 0);
+	fprintf(debug_f, "<5>Started sample_umh\n");
+	loop();
+	fclose(debug_f);
+
+	return 0;
+}
diff --git a/tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c b/tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c
new file mode 100644
index 00000000000..36c0e69e3f7
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/sample_umd/sample_loader.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the UMD Loader (credits: bpfilter).
+ */
+#include <linux/module.h>
+#include <linux/usermode_driver_mgmt.h>
+
+extern char sample_umh_start;
+extern char sample_umh_end;
+extern struct umd_mgmt sample_mgmt_ops;
+
+static int __init load_umh(void)
+{
+	return umd_mgmt_load(&sample_mgmt_ops, &sample_umh_start,
+			     &sample_umh_end);
+}
+
+static void __exit fini_umh(void)
+{
+	umd_mgmt_unload(&sample_mgmt_ops);
+}
+module_init(load_umh);
+module_exit(fini_umh);
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c b/tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c
new file mode 100644
index 00000000000..75c572f9849
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/sample_umd/sample_mgr.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Implement the UMD Manager.
+ */
+#include <linux/module.h>
+#include <linux/security.h>
+#include <linux/seq_file.h>
+#include <linux/usermode_driver_mgmt.h>
+
+#include "msgfmt.h"
+
+struct umd_mgmt sample_mgmt_ops;
+EXPORT_SYMBOL_GPL(sample_mgmt_ops);
+
+struct dentry *sample_umd_dentry;
+
+static int sample_write_common(u32 offset, bool test)
+{
+	struct sample_request *req;
+	struct sample_reply *reply;
+	int ret;
+
+	req = kzalloc(sizeof(*req), GFP_KERNEL);
+	if (!req) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	reply = kzalloc(sizeof(*reply), GFP_KERNEL);
+	if (!reply) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	req->offset = offset;
+
+	if (test)
+		/* Lock is already taken. */
+		ret = umd_send_recv(&sample_mgmt_ops.info, req, sizeof(*req),
+				    reply, sizeof(*reply));
+	else
+		ret = umd_mgmt_send_recv(&sample_mgmt_ops, req, sizeof(*req),
+					 reply, sizeof(*reply));
+	if (ret < 0)
+		goto out;
+
+	if (reply->data[req->offset] != 1) {
+		ret = -EINVAL;
+		goto out;
+	}
+out:
+	kfree(req);
+	kfree(reply);
+
+	return ret;
+}
+
+static ssize_t sample_umd_write(struct file *file, const char __user *buf,
+				size_t datalen, loff_t *ppos)
+{
+	char offset_str[8];
+	u32 offset;
+	int ret;
+
+	if (datalen >= sizeof(offset_str))
+		return -EINVAL;
+
+	ret = copy_from_user(offset_str, buf, datalen);
+	if (ret < 0)
+		return ret;
+
+	offset_str[datalen] = '\0';
+
+	ret = kstrtou32(offset_str, 10, &offset);
+	if (ret < 0)
+		return ret;
+
+	if (offset >= sizeof(((struct sample_reply *)0)->data))
+		return -EINVAL;
+
+	ret = sample_write_common(offset, false);
+	if (ret < 0)
+		return ret;
+
+	return datalen;
+}
+
+static const struct file_operations sample_umd_file_ops = {
+	.write = sample_umd_write,
+};
+
+static int sample_post_start_umh(struct umd_mgmt *mgmt)
+{
+	return sample_write_common(0, true);
+}
+
+static int __init load_umh(void)
+{
+	mutex_init(&sample_mgmt_ops.lock);
+	sample_mgmt_ops.info.tgid = NULL;
+	sample_mgmt_ops.info.driver_name = "sample_umh";
+	sample_mgmt_ops.post_start = sample_post_start_umh;
+	sample_mgmt_ops.kmod = "sample_loader_kmod";
+	sample_mgmt_ops.kmod_loaded = false;
+
+	sample_umd_dentry = securityfs_create_file("sample_umd", 0200, NULL,
+						   NULL, &sample_umd_file_ops);
+	if (IS_ERR(sample_umd_dentry))
+		return PTR_ERR(sample_umd_dentry);
+
+	return 0;
+}
+
+static void __exit fini_umh(void)
+{
+	securityfs_remove(sample_umd_dentry);
+}
+module_init(load_umh);
+module_exit(fini_umh);
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/umd_mgmt/umd_mgmt.sh b/tools/testing/selftests/umd_mgmt/umd_mgmt.sh
new file mode 100755
index 00000000000..9b90d737fec
--- /dev/null
+++ b/tools/testing/selftests/umd_mgmt/umd_mgmt.sh
@@ -0,0 +1,40 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (C) 2023 Huawei Technologies Duesseldorf GmbH
+#
+# Author: Roberto Sassu <roberto.sassu@huawei.com>
+#
+# Script to test the UMD management library.
+
+# Kselftest framework defines: ksft_pass=0, ksft_fail=1, ksft_skip=4
+ksft_pass=0
+ksft_fail=1
+ksft_skip=4
+
+if ! /sbin/modprobe -q sample_mgr; then
+	echo "umd_mgmt: module sample_mgr is not found [SKIP]"
+	exit $ksft_skip
+fi
+
+if [ ! -f /sys/kernel/security/sample_umd ]; then
+	echo "umd_mgmt: kernel interface is not found [SKIP]"
+	exit $ksft_skip
+fi
+
+i=0
+
+while [ $i -lt 500 ]; do
+	if ! echo $(( RANDOM % 128 * 1024 )) > /sys/kernel/security/sample_umd; then
+		echo "umd_mgmt: test failed"
+		exit $ksft_fail
+	fi
+
+	if [ $(( i % 50 )) -eq 0 ]; then
+		rmmod sample_loader_kmod
+	fi
+
+	(( i++ ))
+done
+
+exit $ksft_pass
-- 
2.25.1

[PATCH 5/5] doc: Add documentation for the User Mode Driver management library

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Add a documentation, to explain the motivation behind the new component, to
describe the software architecture, the API and an example used for
testing.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/driver-api/index.rst    |  1 +
 Documentation/driver-api/umd_mgmt.rst | 99 +++++++++++++++++++++++++++
 MAINTAINERS                           |  1 +
 3 files changed, 101 insertions(+)
 create mode 100644 Documentation/driver-api/umd_mgmt.rst

diff --git a/Documentation/driver-api/index.rst b/Documentation/driver-api/index.rst
index ff9aa1afdc6..ad42cd968dc 100644
--- a/Documentation/driver-api/index.rst
+++ b/Documentation/driver-api/index.rst
@@ -113,6 +113,7 @@ available subsections can be seen below.
    xillybus
    zorro
    hte/index
+   umd_mgmt
 
 .. only::  subproject and html
 
diff --git a/Documentation/driver-api/umd_mgmt.rst b/Documentation/driver-api/umd_mgmt.rst
new file mode 100644
index 00000000000..7dbb50b3643
--- /dev/null
+++ b/Documentation/driver-api/umd_mgmt.rst
@@ -0,0 +1,99 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+===================================
+User Mode Driver Management Library
+===================================
+
+:Author: Roberto Sassu, Huawei Technologies Duesseldorf GmbH
+:Date:   2023-03-16
+
+Introduction
+============
+
+The idea of moving code away from the kernel and running it in user space
+is not new. For example, the User Space I/O driver model allows developers
+to implement most of the driver logic in user space, and keep only a small
+part in a kernel module, for example to handle interrupts.
+
+The User Mode Driver (UMD) is a more specialized solution, primarily used
+by bpfilter, consisting of a user space process running from code embedded
+in a kernel module, communicating only through a pipe with the kernel.
+
+The kernel makes a request, possibly originated by the user of the system,
+and sends it to the user space process. The latter handles the kernel
+request, and sends the response back to the kernel. Finally, the kernel
+eventually forwards the result to the user.
+
+This usage model is particularly interesting for security. The kernel can
+offload to user space workloads that could introduce possible threats, for
+example parsing unknown and possibly malicious data. While the kernel
+already does that, it is important to limit to the minimum the chances of
+an attacker to exploit a vulnerability in the kernel code.
+
+If a user space process becomes corrupted, it can still send malicious data
+to the kernel, but it won't be able to directly corrupt the kernel memory.
+In addition, if the communication protocol between the user space process
+and the kernel is simple enough, malicious data can be effectively
+sanitized.
+
+The purpose of this library is simply to facilitate developers to create
+UMDs and to help them customize the UMDs to their needs.
+
+
+
+Architecture
+============
+
+The architecture of the UMD library is as follows:
+
+::
+
+ +-----------+                    +---------------+
+ |    UMD    | 2. request module  |   UMD Loader  |
+ |  Manager  |------------------->| (kmod +       |
+ |           |------+             |  user binary) |
+ +-----------+      |             +---------------+
+       ^            |                    |                     kernel space
+ --------------------------------------------------------------------------
+       |            | 4. send/           v 3. fork/execve/pipe   user space
+       |            |    receive  +-------------+
+ 1. user request    +------------>| UMD Handler |
+                                  | (exec user  |
+                                  |  binary)    |
+                                  +-------------+
+
+The `UMD Manager` is the frontend interface to any user or
+kernel-originated request. It invokes the `UMD Loader` to start the
+`UMD Handler`, and communicates with the latter to satisfy the request.
+
+The `UMD Loader` is merely responsible to extract the `user binary` from
+the kernel module, copy it to a tmpfs filesystem, fork the current process,
+start the `UMD Handler`, and create a pipe for the communication between
+the `UMD Manager` and the `UMD Handler`.
+
+The `UMD Handler` reads requests from the `UMD Manager`, processes them
+internally, and sends the response to it.
+
+
+API
+===
+
+.. kernel-doc:: include/linux/usermode_driver_mgmt.h
+
+.. kernel-doc:: kernel/usermode_driver_mgmt.c
+
+
+Example
+=======
+
+An example of usage of the UMD management library can be found in
+tools/testing/selftests/umd_mgmt/sample_umd.
+
+sample_mgr.c implements the `UMD Manager`, sample_loader.c implements the
+`UMD Loader` and, finally, sample_handler.c implements the `UMD Handler`.
+
+The `UMD Manager` exposes /sys/kernel/security/sample_umd and accepts a
+number between 0-128K intended as an offset in the response buffer, at
+which the `UMD Handler` sets the byte to 1. The `UMD Manager` verifies
+that. If the byte is not set to 1, the `UMD Manager` rejects the write, so
+that the failure can be reported by the test.
diff --git a/MAINTAINERS b/MAINTAINERS
index a0cd161843e..4b9d251259d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11249,6 +11249,7 @@ KERNEL USERMODE DRIVER MANAGEMENT
 M:	Roberto Sassu <roberto.sassu@huawei.com>
 L:	linux-kernel@vger.kernel.org
 S:	Maintained
+F:	Documentation/driver-api/umd_mgmt.rst
 F:	include/linux/usermode_driver_mgmt.h
 F:	kernel/usermode_driver_mgmt.c
 F:	tools/testing/selftests/umd_mgmt/*
-- 
2.25.1

Re: [PATCH 0/5] usermode_driver: Add management library and API

[Alexei Starovoitov]

On Fri, Mar 17, 2023 at 7:53 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
> which runs a user space process from a binary blob, and creates a
> bidirectional pipe, so that the kernel can make a request to that process,
> and the latter provides its response. It is currently used by bpfilter,
> although it does not seem to do any useful work.

FYI the new home for bpfilter is here:
https://github.com/facebook/bpfilter

> The problem is, if other users would like to implement a UMD similar to
> bpfilter, they would have to duplicate the code. Instead, make an UMD
> management library and API from the existing bpfilter and sockopt code,
> and move it to common kernel code.
>
> Also, define the software architecture and the main components of the
> library: the UMD Manager, running in the kernel, acting as the frontend
> interface to any user or kernel-originated request; the UMD Loader, also
> running in the kernel, responsible to load the UMD Handler; the UMD
> Handler, running in user space, responsible to handle requests from the UMD
> Manager and to send to it the response.

That doesn't look like a generic interface for UMD.
It was a quick hack to get bpfilter off the ground, but certainly
not a generic one.

> I have two use cases, but for sake of brevity I will propose one.
>
> I would like to add support for PGP keys and signatures in the kernel, so
> that I can extend secure boot to applications, and allow/deny code
> execution based on the signed file digests included in RPM headers.
>
> While I proposed a patch set a while ago (based on a previous work of David
> Howells), the main objection was that the PGP packet parser should not run
> in the kernel.
>
> That makes a perfect example for using a UMD. If the PGP parser is moved to
> user space (UMD Handler), and the kernel (UMD Manager) just instantiates
> the key and verifies the signature on already parsed data, this would
> address the concern.

I don't think PGP parser belongs to UMD either.
Please do it as a normal user space process and define a proper
protocol for communication between kernel and user space.

Re: [PATCH 0/5] usermode_driver: Add management library and API

[Roberto Sassu]

On Tue, 2023-03-21 at 19:23 -0700, Alexei Starovoitov wrote:
> On Fri, Mar 17, 2023 at 7:53 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
> > which runs a user space process from a binary blob, and creates a
> > bidirectional pipe, so that the kernel can make a request to that process,
> > and the latter provides its response. It is currently used by bpfilter,
> > although it does not seem to do any useful work.
> 
> FYI the new home for bpfilter is here:
> https://github.com/facebook/bpfilter

Thanks. I just ensured that it worked, by doing:

getsockopt(fd, SOL_IP, IPT_SO_GET_INFO, &info, &optlen);

and accepting IPT_SO_GET_INFO in main.c.

> > The problem is, if other users would like to implement a UMD similar to
> > bpfilter, they would have to duplicate the code. Instead, make an UMD
> > management library and API from the existing bpfilter and sockopt code,
> > and move it to common kernel code.
> > 
> > Also, define the software architecture and the main components of the
> > library: the UMD Manager, running in the kernel, acting as the frontend
> > interface to any user or kernel-originated request; the UMD Loader, also
> > running in the kernel, responsible to load the UMD Handler; the UMD
> > Handler, running in user space, responsible to handle requests from the UMD
> > Manager and to send to it the response.
> 
> That doesn't look like a generic interface for UMD.

What would make it more generic? I made the API message format-
independent. It has the capability of starting the user space process
as required, when there is a communication.

> It was a quick hack to get bpfilter off the ground, but certainly
> not a generic one.

True, it is not generic in the sense that it can accomodate any
possible use case. The main goal is to move something that was running
in the kernel to user space, with the same isolation guarantees as if
the code was executed in the kernel.

> > I have two use cases, but for sake of brevity I will propose one.
> > 
> > I would like to add support for PGP keys and signatures in the kernel, so
> > that I can extend secure boot to applications, and allow/deny code
> > execution based on the signed file digests included in RPM headers.
> > 
> > While I proposed a patch set a while ago (based on a previous work of David
> > Howells), the main objection was that the PGP packet parser should not run
> > in the kernel.
> > 
> > That makes a perfect example for using a UMD. If the PGP parser is moved to
> > user space (UMD Handler), and the kernel (UMD Manager) just instantiates
> > the key and verifies the signature on already parsed data, this would
> > address the concern.
> 
> I don't think PGP parser belongs to UMD either.
> Please do it as a normal user space process and define a proper
> protocol for communication between kernel and user space.

UMD is better in the sense that it establishes a bidirectional pipe
between the kernel and the user space process. With that, there is no
need to further restrict the access to a sysfs file, for example.

The UMD mechanism is much more effective: the pipe is already
established with the right process, whose code was integrity-checked
because embedded in the kernel module.

In addition to that, I'm using seccomp to further restrict what the
user space process can do (read, write, exit, ...). That process cannot
open new communication channels, even if corrupted. It is expected to
send to the kernel simple data structures, that the kernel can
effectively sanitize.

The last step to achieve full isolation would be to deny ptrace/kill on
the user space process created by the UMD management library so that,
in lockdown mode, not even root can interfer with that process.

Roberto

Re: [PATCH 5/5] doc: Add documentation for the User Mode Driver management library

[Bagas Sanjaya]

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

On Fri, Mar 17, 2023 at 03:52:40PM +0100, Roberto Sassu wrote:
> +The `UMD Manager` is the frontend interface to any user or
> +kernel-originated request. It invokes the `UMD Loader` to start the
> +`UMD Handler`, and communicates with the latter to satisfy the request.
> +
> +The `UMD Loader` is merely responsible to extract the `user binary` from
> +the kernel module, copy it to a tmpfs filesystem, fork the current process,
> +start the `UMD Handler`, and create a pipe for the communication between
> +the `UMD Manager` and the `UMD Handler`.
> +
> +The `UMD Handler` reads requests from the `UMD Manager`, processes them
> +internally, and sends the response to it.

I think you can write out the full forms (UMD manager, UMD loader, and
UMD handler) once and for subsequent mentions of these, UMD can be
omitted, since the manager/loader/handler will obviously refers to the
UMD one.

Otherwise LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 0/5] usermode_driver: Add management library and API

[Alexei Starovoitov]

On Wed, Mar 22, 2023 at 5:08 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> On Tue, 2023-03-21 at 19:23 -0700, Alexei Starovoitov wrote:
> > On Fri, Mar 17, 2023 at 7:53 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > >
> > > A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
> > > which runs a user space process from a binary blob, and creates a
> > > bidirectional pipe, so that the kernel can make a request to that process,
> > > and the latter provides its response. It is currently used by bpfilter,
> > > although it does not seem to do any useful work.
> >
> > FYI the new home for bpfilter is here:
> > https://github.com/facebook/bpfilter
>
> Thanks. I just ensured that it worked, by doing:
>
> getsockopt(fd, SOL_IP, IPT_SO_GET_INFO, &info, &optlen);
>
> and accepting IPT_SO_GET_INFO in main.c.
>
> > > The problem is, if other users would like to implement a UMD similar to
> > > bpfilter, they would have to duplicate the code. Instead, make an UMD
> > > management library and API from the existing bpfilter and sockopt code,
> > > and move it to common kernel code.
> > >
> > > Also, define the software architecture and the main components of the
> > > library: the UMD Manager, running in the kernel, acting as the frontend
> > > interface to any user or kernel-originated request; the UMD Loader, also
> > > running in the kernel, responsible to load the UMD Handler; the UMD
> > > Handler, running in user space, responsible to handle requests from the UMD
> > > Manager and to send to it the response.
> >
> > That doesn't look like a generic interface for UMD.
>
> What would make it more generic? I made the API message format-
> independent. It has the capability of starting the user space process
> as required, when there is a communication.
>
> > It was a quick hack to get bpfilter off the ground, but certainly
> > not a generic one.
>
> True, it is not generic in the sense that it can accomodate any
> possible use case. The main goal is to move something that was running
> in the kernel to user space, with the same isolation guarantees as if
> the code was executed in the kernel.

They are not the same guarantees.
UMD is exactly equivalent to root process running in user space.
Meaning it can be killed, ptraced, priority inverted, etc

> > > I have two use cases, but for sake of brevity I will propose one.
> > >
> > > I would like to add support for PGP keys and signatures in the kernel, so
> > > that I can extend secure boot to applications, and allow/deny code
> > > execution based on the signed file digests included in RPM headers.
> > >
> > > While I proposed a patch set a while ago (based on a previous work of David
> > > Howells), the main objection was that the PGP packet parser should not run
> > > in the kernel.
> > >
> > > That makes a perfect example for using a UMD. If the PGP parser is moved to
> > > user space (UMD Handler), and the kernel (UMD Manager) just instantiates
> > > the key and verifies the signature on already parsed data, this would
> > > address the concern.
> >
> > I don't think PGP parser belongs to UMD either.
> > Please do it as a normal user space process and define a proper
> > protocol for communication between kernel and user space.
>
> UMD is better in the sense that it establishes a bidirectional pipe
> between the kernel and the user space process. With that, there is no
> need to further restrict the access to a sysfs file, for example.

If a simple pipe is good enough then you can have a kernel module
that creates it and interacts with the user space process.
Out-of-tree bpftiler can do that, so can you.
PGP is not suitable for kernel git repo either as kernel code or as UMD.

Re: [PATCH 0/5] usermode_driver: Add management library and API

[Roberto Sassu]

On Wed, 2023-03-22 at 15:27 -0700, Alexei Starovoitov wrote:
> On Wed, Mar 22, 2023 at 5:08 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Tue, 2023-03-21 at 19:23 -0700, Alexei Starovoitov wrote:
> > > On Fri, Mar 17, 2023 at 7:53 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > 
> > > > A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
> > > > which runs a user space process from a binary blob, and creates a
> > > > bidirectional pipe, so that the kernel can make a request to that process,
> > > > and the latter provides its response. It is currently used by bpfilter,
> > > > although it does not seem to do any useful work.
> > > 
> > > FYI the new home for bpfilter is here:
> > > https://github.com/facebook/bpfilter
> > 
> > Thanks. I just ensured that it worked, by doing:
> > 
> > getsockopt(fd, SOL_IP, IPT_SO_GET_INFO, &info, &optlen);
> > 
> > and accepting IPT_SO_GET_INFO in main.c.
> > 
> > > > The problem is, if other users would like to implement a UMD similar to
> > > > bpfilter, they would have to duplicate the code. Instead, make an UMD
> > > > management library and API from the existing bpfilter and sockopt code,
> > > > and move it to common kernel code.
> > > > 
> > > > Also, define the software architecture and the main components of the
> > > > library: the UMD Manager, running in the kernel, acting as the frontend
> > > > interface to any user or kernel-originated request; the UMD Loader, also
> > > > running in the kernel, responsible to load the UMD Handler; the UMD
> > > > Handler, running in user space, responsible to handle requests from the UMD
> > > > Manager and to send to it the response.
> > > 
> > > That doesn't look like a generic interface for UMD.
> > 
> > What would make it more generic? I made the API message format-
> > independent. It has the capability of starting the user space process
> > as required, when there is a communication.
> > 
> > > It was a quick hack to get bpfilter off the ground, but certainly
> > > not a generic one.
> > 
> > True, it is not generic in the sense that it can accomodate any
> > possible use case. The main goal is to move something that was running
> > in the kernel to user space, with the same isolation guarantees as if
> > the code was executed in the kernel.
> 
> They are not the same guarantees.
> UMD is exactly equivalent to root process running in user space.
> Meaning it can be killed, ptraced, priority inverted, etc

That is the starting point.

I suppose you can remove any privilege from the UMD process, it just
needs to read/write from/to a pipe (and in my case to use socket() with
AF_ALG to interact with the Crypto API).

Also, as I mentioned, you can enforce a very strict seccomp profile,
which forces the UMD process to use a very limited number of system
calls.

For the interactions of the rest of the system to the UMD process, you
could deny with an LSM all the operations that you mentioned. The rest
of the system would not be affected, only operations which have the UMD
process as target are denied.

> > > > I have two use cases, but for sake of brevity I will propose one.
> > > > 
> > > > I would like to add support for PGP keys and signatures in the kernel, so
> > > > that I can extend secure boot to applications, and allow/deny code
> > > > execution based on the signed file digests included in RPM headers.
> > > > 
> > > > While I proposed a patch set a while ago (based on a previous work of David
> > > > Howells), the main objection was that the PGP packet parser should not run
> > > > in the kernel.
> > > > 
> > > > That makes a perfect example for using a UMD. If the PGP parser is moved to
> > > > user space (UMD Handler), and the kernel (UMD Manager) just instantiates
> > > > the key and verifies the signature on already parsed data, this would
> > > > address the concern.
> > > 
> > > I don't think PGP parser belongs to UMD either.
> > > Please do it as a normal user space process and define a proper
> > > protocol for communication between kernel and user space.
> > 
> > UMD is better in the sense that it establishes a bidirectional pipe
> > between the kernel and the user space process. With that, there is no
> > need to further restrict the access to a sysfs file, for example.
> 
> If a simple pipe is good enough then you can have a kernel module
> that creates it and interacts with the user space process.

Few points I forgot to mention.

With the UMD approach, the binary blob is embedded in the kernel
module, which means that no external dependencies are needed for
integrity verification. The binary is statically compiled, and the
kernel write-protects it at run-time.

Second, since DIGLIM would check the integrity of any executable,
including init, the PGP signature verification needs to occur before.
So, the PGP UMD should be already started by then. That is not going to
be a problem, since the binary is copied to a private tmpfs mount.

> Out-of-tree bpftiler can do that, so can you.

As far as I can see, the out-of-tree bpfilter works exactly in the same
way as the in-tree counterpart. The binary blob is embedded in the
kernel module.

> PGP is not suitable for kernel git repo either as kernel code or as UMD.

Well, the asymmetric key type can be extended with new parsers, so this
possibility was already taken into account. The objection that the PGP
parser should not run in kernel space is fair, but I think the UMD
approach fully addresses that.

Also, I agree with you that we should not just take any code and
pretend that it is part of the kernel. However, in this particular
case, the purpose of the PGP UMD would be simply to extract very few
information from the PGP packets. The asymmetric key type and the
signature verification infrastructure already take care of the rest.

PGP keys and signatures would act as an additional system trust anchor
for verifying critical system data (for DIGLIM, which executables are
allowed to run), similarly to how X.509 certificates are used for
verifying kernel modules. RPM headers, executables digests are taken
from, are signed with PGP, so there is no other way than adding this
functionality.

And unfortunately, especially for features impacting the entire system,
out-of-tree drivers are not really an option:

https://docs.fedoraproject.org/en-US/quick-docs/kernel/overview/#_out_of_tree_drivers

Thanks

Roberto

Re: [PATCH 0/5] usermode_driver: Add management library and API

[Alexei Starovoitov]

On Thu, Mar 23, 2023 at 6:37 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
>
> On Wed, 2023-03-22 at 15:27 -0700, Alexei Starovoitov wrote:
> > On Wed, Mar 22, 2023 at 5:08 AM Roberto Sassu
> > <roberto.sassu@huaweicloud.com> wrote:
> > > On Tue, 2023-03-21 at 19:23 -0700, Alexei Starovoitov wrote:
> > > > On Fri, Mar 17, 2023 at 7:53 AM Roberto Sassu
> > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > >
> > > > > A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
> > > > > which runs a user space process from a binary blob, and creates a
> > > > > bidirectional pipe, so that the kernel can make a request to that process,
> > > > > and the latter provides its response. It is currently used by bpfilter,
> > > > > although it does not seem to do any useful work.
> > > >
> > > > FYI the new home for bpfilter is here:
> > > > https://github.com/facebook/bpfilter
> > >
> > > Thanks. I just ensured that it worked, by doing:
> > >
> > > getsockopt(fd, SOL_IP, IPT_SO_GET_INFO, &info, &optlen);
> > >
> > > and accepting IPT_SO_GET_INFO in main.c.
> > >
> > > > > The problem is, if other users would like to implement a UMD similar to
> > > > > bpfilter, they would have to duplicate the code. Instead, make an UMD
> > > > > management library and API from the existing bpfilter and sockopt code,
> > > > > and move it to common kernel code.
> > > > >
> > > > > Also, define the software architecture and the main components of the
> > > > > library: the UMD Manager, running in the kernel, acting as the frontend
> > > > > interface to any user or kernel-originated request; the UMD Loader, also
> > > > > running in the kernel, responsible to load the UMD Handler; the UMD
> > > > > Handler, running in user space, responsible to handle requests from the UMD
> > > > > Manager and to send to it the response.
> > > >
> > > > That doesn't look like a generic interface for UMD.
> > >
> > > What would make it more generic? I made the API message format-
> > > independent. It has the capability of starting the user space process
> > > as required, when there is a communication.
> > >
> > > > It was a quick hack to get bpfilter off the ground, but certainly
> > > > not a generic one.
> > >
> > > True, it is not generic in the sense that it can accomodate any
> > > possible use case. The main goal is to move something that was running
> > > in the kernel to user space, with the same isolation guarantees as if
> > > the code was executed in the kernel.
> >
> > They are not the same guarantees.
> > UMD is exactly equivalent to root process running in user space.
> > Meaning it can be killed, ptraced, priority inverted, etc
>
> That is the starting point.
>
> I suppose you can remove any privilege from the UMD process, it just
> needs to read/write from/to a pipe (and in my case to use socket() with
> AF_ALG to interact with the Crypto API).
>
> Also, as I mentioned, you can enforce a very strict seccomp profile,
> which forces the UMD process to use a very limited number of system
> calls.
>
> For the interactions of the rest of the system to the UMD process, you
> could deny with an LSM all the operations that you mentioned. The rest
> of the system would not be affected, only operations which have the UMD
> process as target are denied.
>
> > > > > I have two use cases, but for sake of brevity I will propose one.
> > > > >
> > > > > I would like to add support for PGP keys and signatures in the kernel, so
> > > > > that I can extend secure boot to applications, and allow/deny code
> > > > > execution based on the signed file digests included in RPM headers.
> > > > >
> > > > > While I proposed a patch set a while ago (based on a previous work of David
> > > > > Howells), the main objection was that the PGP packet parser should not run
> > > > > in the kernel.
> > > > >
> > > > > That makes a perfect example for using a UMD. If the PGP parser is moved to
> > > > > user space (UMD Handler), and the kernel (UMD Manager) just instantiates
> > > > > the key and verifies the signature on already parsed data, this would
> > > > > address the concern.
> > > >
> > > > I don't think PGP parser belongs to UMD either.
> > > > Please do it as a normal user space process and define a proper
> > > > protocol for communication between kernel and user space.
> > >
> > > UMD is better in the sense that it establishes a bidirectional pipe
> > > between the kernel and the user space process. With that, there is no
> > > need to further restrict the access to a sysfs file, for example.
> >
> > If a simple pipe is good enough then you can have a kernel module
> > that creates it and interacts with the user space process.
>
> Few points I forgot to mention.
>
> With the UMD approach, the binary blob is embedded in the kernel
> module, which means that no external dependencies are needed for
> integrity verification. The binary is statically compiled, and the
> kernel write-protects it at run-time.
>
> Second, since DIGLIM would check the integrity of any executable,
> including init, the PGP signature verification needs to occur before.
> So, the PGP UMD should be already started by then. That is not going to
> be a problem, since the binary is copied to a private tmpfs mount.
>
> > Out-of-tree bpftiler can do that, so can you.
>
> As far as I can see, the out-of-tree bpfilter works exactly in the same
> way as the in-tree counterpart. The binary blob is embedded in the
> kernel module.
>
> > PGP is not suitable for kernel git repo either as kernel code or as UMD.
>
> Well, the asymmetric key type can be extended with new parsers, so this
> possibility was already taken into account. The objection that the PGP
> parser should not run in kernel space is fair, but I think the UMD
> approach fully addresses that.
>
> Also, I agree with you that we should not just take any code and
> pretend that it is part of the kernel. However, in this particular
> case, the purpose of the PGP UMD would be simply to extract very few
> information from the PGP packets. The asymmetric key type and the
> signature verification infrastructure already take care of the rest.
>
> PGP keys and signatures would act as an additional system trust anchor
> for verifying critical system data (for DIGLIM, which executables are
> allowed to run), similarly to how X.509 certificates are used for
> verifying kernel modules. RPM headers, executables digests are taken
> from, are signed with PGP, so there is no other way than adding this
> functionality.
>
> And unfortunately, especially for features impacting the entire system,
> out-of-tree drivers are not really an option:

I think you have to start out of tree and prove that the PGP thing
is worth considering at all.
Only then we can talk about merits of UMD and generalization
of pipe interface if it's applicable.

DIGLIM and everything else you mentioned above doesn't add weight
to the decision. PGP work should be acceptable on its own.
Out-of-tree is a method to prove that it works and later argue
for inclusion as in-tree either as kernel module or UMD.
Generalization of current bpfilter is out of scope here.

Re: [PATCH 0/5] usermode_driver: Add management library and API

[Roberto Sassu]

On Fri, 2023-03-24 at 19:54 -0700, Alexei Starovoitov wrote:
> On Thu, Mar 23, 2023 at 6:37 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Wed, 2023-03-22 at 15:27 -0700, Alexei Starovoitov wrote:
> > > On Wed, Mar 22, 2023 at 5:08 AM Roberto Sassu
> > > <roberto.sassu@huaweicloud.com> wrote:
> > > > On Tue, 2023-03-21 at 19:23 -0700, Alexei Starovoitov wrote:
> > > > > On Fri, Mar 17, 2023 at 7:53 AM Roberto Sassu
> > > > > <roberto.sassu@huaweicloud.com> wrote:
> > > > > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > > 
> > > > > > A User Mode Driver (UMD) is a specialization of a User Mode Helper (UMH),
> > > > > > which runs a user space process from a binary blob, and creates a
> > > > > > bidirectional pipe, so that the kernel can make a request to that process,
> > > > > > and the latter provides its response. It is currently used by bpfilter,
> > > > > > although it does not seem to do any useful work.
> > > > > 
> > > > > FYI the new home for bpfilter is here:
> > > > > https://github.com/facebook/bpfilter
> > > > 
> > > > Thanks. I just ensured that it worked, by doing:
> > > > 
> > > > getsockopt(fd, SOL_IP, IPT_SO_GET_INFO, &info, &optlen);
> > > > 
> > > > and accepting IPT_SO_GET_INFO in main.c.
> > > > 
> > > > > > The problem is, if other users would like to implement a UMD similar to
> > > > > > bpfilter, they would have to duplicate the code. Instead, make an UMD
> > > > > > management library and API from the existing bpfilter and sockopt code,
> > > > > > and move it to common kernel code.
> > > > > > 
> > > > > > Also, define the software architecture and the main components of the
> > > > > > library: the UMD Manager, running in the kernel, acting as the frontend
> > > > > > interface to any user or kernel-originated request; the UMD Loader, also
> > > > > > running in the kernel, responsible to load the UMD Handler; the UMD
> > > > > > Handler, running in user space, responsible to handle requests from the UMD
> > > > > > Manager and to send to it the response.
> > > > > 
> > > > > That doesn't look like a generic interface for UMD.
> > > > 
> > > > What would make it more generic? I made the API message format-
> > > > independent. It has the capability of starting the user space process
> > > > as required, when there is a communication.
> > > > 
> > > > > It was a quick hack to get bpfilter off the ground, but certainly
> > > > > not a generic one.
> > > > 
> > > > True, it is not generic in the sense that it can accomodate any
> > > > possible use case. The main goal is to move something that was running
> > > > in the kernel to user space, with the same isolation guarantees as if
> > > > the code was executed in the kernel.
> > > 
> > > They are not the same guarantees.
> > > UMD is exactly equivalent to root process running in user space.
> > > Meaning it can be killed, ptraced, priority inverted, etc
> > 
> > That is the starting point.
> > 
> > I suppose you can remove any privilege from the UMD process, it just
> > needs to read/write from/to a pipe (and in my case to use socket() with
> > AF_ALG to interact with the Crypto API).
> > 
> > Also, as I mentioned, you can enforce a very strict seccomp profile,
> > which forces the UMD process to use a very limited number of system
> > calls.
> > 
> > For the interactions of the rest of the system to the UMD process, you
> > could deny with an LSM all the operations that you mentioned. The rest
> > of the system would not be affected, only operations which have the UMD
> > process as target are denied.
> > 
> > > > > > I have two use cases, but for sake of brevity I will propose one.
> > > > > > 
> > > > > > I would like to add support for PGP keys and signatures in the kernel, so
> > > > > > that I can extend secure boot to applications, and allow/deny code
> > > > > > execution based on the signed file digests included in RPM headers.
> > > > > > 
> > > > > > While I proposed a patch set a while ago (based on a previous work of David
> > > > > > Howells), the main objection was that the PGP packet parser should not run
> > > > > > in the kernel.
> > > > > > 
> > > > > > That makes a perfect example for using a UMD. If the PGP parser is moved to
> > > > > > user space (UMD Handler), and the kernel (UMD Manager) just instantiates
> > > > > > the key and verifies the signature on already parsed data, this would
> > > > > > address the concern.
> > > > > 
> > > > > I don't think PGP parser belongs to UMD either.
> > > > > Please do it as a normal user space process and define a proper
> > > > > protocol for communication between kernel and user space.
> > > > 
> > > > UMD is better in the sense that it establishes a bidirectional pipe
> > > > between the kernel and the user space process. With that, there is no
> > > > need to further restrict the access to a sysfs file, for example.
> > > 
> > > If a simple pipe is good enough then you can have a kernel module
> > > that creates it and interacts with the user space process.
> > 
> > Few points I forgot to mention.
> > 
> > With the UMD approach, the binary blob is embedded in the kernel
> > module, which means that no external dependencies are needed for
> > integrity verification. The binary is statically compiled, and the
> > kernel write-protects it at run-time.
> > 
> > Second, since DIGLIM would check the integrity of any executable,
> > including init, the PGP signature verification needs to occur before.
> > So, the PGP UMD should be already started by then. That is not going to
> > be a problem, since the binary is copied to a private tmpfs mount.
> > 
> > > Out-of-tree bpftiler can do that, so can you.
> > 
> > As far as I can see, the out-of-tree bpfilter works exactly in the same
> > way as the in-tree counterpart. The binary blob is embedded in the
> > kernel module.
> > 
> > > PGP is not suitable for kernel git repo either as kernel code or as UMD.
> > 
> > Well, the asymmetric key type can be extended with new parsers, so this
> > possibility was already taken into account. The objection that the PGP
> > parser should not run in kernel space is fair, but I think the UMD
> > approach fully addresses that.
> > 
> > Also, I agree with you that we should not just take any code and
> > pretend that it is part of the kernel. However, in this particular
> > case, the purpose of the PGP UMD would be simply to extract very few
> > information from the PGP packets. The asymmetric key type and the
> > signature verification infrastructure already take care of the rest.
> > 
> > PGP keys and signatures would act as an additional system trust anchor
> > for verifying critical system data (for DIGLIM, which executables are
> > allowed to run), similarly to how X.509 certificates are used for
> > verifying kernel modules. RPM headers, executables digests are taken
> > from, are signed with PGP, so there is no other way than adding this
> > functionality.
> > 
> > And unfortunately, especially for features impacting the entire system,
> > out-of-tree drivers are not really an option:
> 
> I think you have to start out of tree and prove that the PGP thing
> is worth considering at all.
> Only then we can talk about merits of UMD and generalization
> of pipe interface if it's applicable.

Ok. This is my plan:

1) send the kernel part necessary to support PGP keys and signatures
2) evaluate/discuss if something else can be moved to user space
3) propose the implementation of the PGP UMD
4) assess the robustness of the solution, and compare to different
   designs

Thanks

Roberto

> DIGLIM and everything else you mentioned above doesn't add weight
> to the decision. PGP work should be acceptable on its own.
> Out-of-tree is a method to prove that it works and later argue
> for inclusion as in-tree either as kernel module or UMD.
> Generalization of current bpfilter is out of scope here.