From: Roberto Sassu <roberto.sassu@huawei.com>
Introduce ksys_finit_module() to let kernel components request a kernel
module without requiring running modprobe.
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
include/linux/syscalls.h | 10 ++++++++++
kernel/module/main.c | 43 ++++++++++++++++++++++++++++++----------
2 files changed, 43 insertions(+), 10 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 5758104921e6..18bb346bb793 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -1233,6 +1233,16 @@ int ksys_ipc(unsigned int call, int first, unsigned long second,
int compat_ksys_ipc(u32 call, int first, int second,
u32 third, u32 ptr, u32 fifth);
+#ifdef CONFIG_MODULES
+int ksys_finit_module(struct file *f, const char *kargs, int flags);
+#else
+static inline int ksys_finit_module(struct file *f, const char *kargs,
+ int flags)
+{
+ return -EOPNOTSUPP;
+}
+#endif
+
/*
* The following kernel syscall equivalents are just wrappers to fs-internal
* functions. Therefore, provide stubs to be inlined at the callsites.
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 49b9bca9de12..81f79c9ea637 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2852,7 +2852,7 @@ static int early_mod_check(struct load_info *info, int flags)
* zero, and we rely on this for optional sections.
*/
static int load_module(struct load_info *info, const char __user *uargs,
- int flags)
+ const char *kargs, int flags)
{
struct module *mod;
bool module_allocated = false;
@@ -2953,7 +2953,13 @@ static int load_module(struct load_info *info, const char __user *uargs,
flush_module_icache(mod);
/* Now copy in args */
- mod->args = strndup_user(uargs, ~0UL >> 1);
+ if (kargs) {
+ mod->args = kstrndup(kargs, ~0UL >> 1, GFP_KERNEL);
+ if (!mod->args)
+ mod->args = ERR_PTR(-ENOMEM);
+ } else {
+ mod->args = strndup_user(uargs, ~0UL >> 1);
+ }
if (IS_ERR(mod->args)) {
err = PTR_ERR(mod->args);
goto free_arch_cleanup;
@@ -3083,7 +3089,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
return err;
}
- return load_module(&info, uargs, 0);
+ return load_module(&info, uargs, NULL, 0);
}
struct idempotent {
@@ -3170,7 +3176,8 @@ static int idempotent_wait_for_completion(struct idempotent *u)
return u->ret;
}
-static int init_module_from_file(struct file *f, const char __user * uargs, int flags)
+static int init_module_from_file(struct file *f, const char __user * uargs,
+ const char *kargs, int flags)
{
struct load_info info = { };
void *buf = NULL;
@@ -3195,10 +3202,11 @@ static int init_module_from_file(struct file *f, const char __user * uargs, int
info.len = len;
}
- return load_module(&info, uargs, flags);
+ return load_module(&info, uargs, kargs, flags);
}
-static int idempotent_init_module(struct file *f, const char __user * uargs, int flags)
+static int idempotent_init_module(struct file *f, const char __user * uargs,
+ const char *kargs, int flags)
{
struct idempotent idem;
@@ -3207,7 +3215,7 @@ static int idempotent_init_module(struct file *f, const char __user * uargs, int
/* Are we the winners of the race and get to do this? */
if (!idempotent(&idem, file_inode(f))) {
- int ret = init_module_from_file(f, uargs, flags);
+ int ret = init_module_from_file(f, uargs, kargs, flags);
return idempotent_complete(&idem, ret);
}
@@ -3217,15 +3225,16 @@ static int idempotent_init_module(struct file *f, const char __user * uargs, int
return idempotent_wait_for_completion(&idem);
}
-SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
+static int _ksys_finit_module(struct file *f, int fd, const char __user * uargs,
+ const char *kargs, int flags)
{
int err;
- struct fd f;
err = may_init_module();
if (err)
return err;
+ /* fd = -1 if called from the kernel. */
pr_debug("finit_module: fd=%d, uargs=%p, flags=%i\n", fd, uargs, flags);
if (flags & ~(MODULE_INIT_IGNORE_MODVERSIONS
@@ -3233,8 +3242,22 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
|MODULE_INIT_COMPRESSED_FILE))
return -EINVAL;
+ err = idempotent_init_module(f, uargs, kargs, flags);
+ return err;
+}
+
+int ksys_finit_module(struct file *f, const char *kargs, int flags)
+{
+ return _ksys_finit_module(f, -1, NULL, kargs, flags);
+}
+
+SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
+{
+ int err;
+ struct fd f;
+
f = fdget(fd);
- err = idempotent_init_module(fd_file(f), uargs, flags);
+ err = _ksys_finit_module(fd_file(f), fd, uargs, NULL, flags);
fdput(f);
return err;
}
--
2.47.0.118.gfd3785337bOn Tue, Nov 19, 2024 at 11:49:09AM +0100, Roberto Sassu wrote: > From: Roberto Sassu <roberto.sassu@huawei.com> > > Introduce ksys_finit_module() to let kernel components request a kernel > module without requiring running modprobe. That does sound more than sketchy, even more so because the commit log completely fails to explain why you'd need to do that.
On Tue, Nov 19, 2024 at 01:14:02PM +0100, Christoph Hellwig wrote: > On Tue, Nov 19, 2024 at 11:49:09AM +0100, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > Introduce ksys_finit_module() to let kernel components request a kernel > > module without requiring running modprobe. > > That does sound more than sketchy, even more so because the commit log > completely fails to explain why you'd need to do that. I also don't think the commit log is correct, I don't see how the code is preventing calling modprobe, the indepotent check is intended to prevent duplicate module init calls which may allocate extra vmalloc space only to release it. You can test to see if your patch has any improvments by enabling MODULE_STATS and MODULE_DEBUG_AUTOLOAD_DUPS and check before / after results of /sys/kernel/debug/modules/stats , right now this patch and commit log is not telling me anything useful. Luis
On Tue, 2024-11-19 at 12:10 -0800, Luis Chamberlain wrote: > On Tue, Nov 19, 2024 at 01:14:02PM +0100, Christoph Hellwig wrote: > > On Tue, Nov 19, 2024 at 11:49:09AM +0100, Roberto Sassu wrote: > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > Introduce ksys_finit_module() to let kernel components request a kernel > > > module without requiring running modprobe. > > > > That does sound more than sketchy, even more so because the commit log > > completely fails to explain why you'd need to do that. > > I also don't think the commit log is correct, I don't see how the > code is preventing calling modprobe, the indepotent check is intended > to prevent duplicate module init calls which may allocate extra vmalloc > space only to release it. You can test to see if your patch has any > improvments by enabling MODULE_STATS and MODULE_DEBUG_AUTOLOAD_DUPS > and check before / after results of /sys/kernel/debug/modules/stats , > right now this patch and commit log is not telling me anything useful. Maybe I misunderstood the code, but what causes modprobe to be executed in user space is a call to request_module(). In my patch, I simply ported the code of the finit_module() system call to _ksys_finit_module(), net the conversion from struct fd to struct file, which is kept in the system call code. Also, from the kernel side, I'm providing a valid address for module arguments, and duplicating the string either with kmemdup() or strndup_user() in load_module(), depending on where the memory belongs to. Again, maybe I misunderstood, but I'm not introducing any functional change to the current behavior, the kernel side also provides a file descriptor and module arguments as user space would do (e.g. by executing insmod). As for the motivation, please have a look at my response to Christian: https://lore.kernel.org/linux-integrity/ZzzvAPetAn7CUEvx@bombadil.infradead.org/T/#ma8656b921bb5bfb60e7f10331061d462a87ce9f4 In addition, you could also see how ksys_finit_module() is used here: https://lore.kernel.org/linux-integrity/20241119104922.2772571-8-roberto.sassu@huaweicloud.com/ Thanks Roberto
On Wed, 2024-11-20 at 10:16 +0100, Roberto Sassu wrote: > On Tue, 2024-11-19 at 12:10 -0800, Luis Chamberlain wrote: > > On Tue, Nov 19, 2024 at 01:14:02PM +0100, Christoph Hellwig wrote: > > > On Tue, Nov 19, 2024 at 11:49:09AM +0100, Roberto Sassu wrote: > > > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > > > > > Introduce ksys_finit_module() to let kernel components request a kernel > > > > module without requiring running modprobe. > > > > > > That does sound more than sketchy, even more so because the commit log > > > completely fails to explain why you'd need to do that. > > > > I also don't think the commit log is correct, I don't see how the > > code is preventing calling modprobe, the indepotent check is intended > > to prevent duplicate module init calls which may allocate extra vmalloc > > space only to release it. You can test to see if your patch has any > > improvments by enabling MODULE_STATS and MODULE_DEBUG_AUTOLOAD_DUPS > > and check before / after results of /sys/kernel/debug/modules/stats , > > right now this patch and commit log is not telling me anything useful. > > Maybe I misunderstood the code, but what causes modprobe to be executed > in user space is a call to request_module(). > > In my patch, I simply ported the code of the finit_module() system call > to _ksys_finit_module(), net the conversion from struct fd to struct > file, which is kept in the system call code. > > Also, from the kernel side, I'm providing a valid address for module > arguments, and duplicating the string either with kmemdup() or > strndup_user() in load_module(), depending on where the memory belongs > to. > > Again, maybe I misunderstood, but I'm not introducing any functional > change to the current behavior, the kernel side also provides a file > descriptor and module arguments as user space would do (e.g. by > executing insmod). > > As for the motivation, please have a look at my response to Christian: Christoph, of course. Roberto > https://lore.kernel.org/linux-integrity/ZzzvAPetAn7CUEvx@bombadil.infradead.org/T/#ma8656b921bb5bfb60e7f10331061d462a87ce9f4 > > > In addition, you could also see how ksys_finit_module() is used here: > > https://lore.kernel.org/linux-integrity/20241119104922.2772571-8-roberto.sassu@huaweicloud.com/ > > Thanks > > Roberto
On Tue, 2024-11-19 at 13:14 +0100, Christoph Hellwig wrote: > On Tue, Nov 19, 2024 at 11:49:09AM +0100, Roberto Sassu wrote: > > From: Roberto Sassu <roberto.sassu@huawei.com> > > > > Introduce ksys_finit_module() to let kernel components request a kernel > > module without requiring running modprobe. > > That does sound more than sketchy, even more so because the commit log > completely fails to explain why you'd need to do that. With my solution, the kernel grants access to a file in user space depending on whether or not its calculated (or fsverity) digest is found in an application manifest provided by the software vendor. However, what it happens is that in the early boot phase the parser is not loaded yet, and the kernel cannot extract the reference digests from the application manifest. Thus, calling request_module() and consequently executing modprobe will fail, since the kernel does not have its reference digest yet. Instead, loading the kernel module from the kernel itself works, because only the kernel module needs to be verified, and that can be done through its appended signature. Roberto
© 2016 - 2024 Red Hat, Inc.