On Fri, Mar 24, 2023 at 1:00 PM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
>
> On the modules side of things we can be super defensive on the second
> vmalloc allocation defensive [0] but other than this the initial kread
> also needs care too.
Please don't re-implement semaphores. They are a *classic* concurrency limiter.
In fact, probably *THE* classic one.
So just do something like this instead:
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2937,6 +2937,11 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
return load_module(&info, uargs, 0);
}
+#define CONCURRENCY_LIMITER(name, n) \
+ struct semaphore name = __SEMAPHORE_INITIALIZER(name, n)
+
+static CONCURRENCY_LIMITER(module_loading_concurrency, 50);
+
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs,
int, flags)
{
struct load_info info = { };
@@ -2955,8 +2960,12 @@ SYSCALL_DEFINE3(finit_module, int, fd, const
char __user *, uargs, int, flags)
|MODULE_INIT_COMPRESSED_FILE))
return -EINVAL;
+ err = down_killable(&module_loading_concurrency);
+ if (err)
+ return err;
len = kernel_read_file_from_fd(fd, 0, &buf, INT_MAX, NULL,
READING_MODULE);
+ up(&module_loading_concurrency);
if (len < 0)
return len;
NOTE! Entirely untested. Surprise surprise.
I'm a tiny bit worried about deadlocks here, so somebody needs to make
sure that the kernel_read_file_from_fd() case cannot possibly in turn
cause a "request_module()" recursion.
We most definitely have had module recursion before, but I *think*
it's always just in the module init function (ie one module requests
another when ithe mod->init() function is called).
I think by the time we have opened the module file descriptors and are
just reading the data, we should be ok and the above would never
deadlock, but I want people to at least think about it.
Of course, with that "max 50 concurrent loaders" limit, maybe it's
never an issue anyway. Even if you get a recursion a few deep, at most
you just wait for somebody else to get out of their loaders. Unless
*everybody* ends up waiting on some progress.
Linus
