kevin/linux
1
Fork 0
Code

module: make waiting for a concurrent module loader interruptible

Browse Source 
The recursive aes-arm-bs module load situation reported by Russell King
is getting fixed in the crypto layer, but this in the meantime fixes the
"recursive load hangs forever" by just making the waiting for the first
module load be interruptible.

This should now match the old behavior before commit 9b9879fc03
("modules: catch concurrent module loads, treat them as idempotent"),
which used the different "wait for module to be ready" code in
module_patient_check_exists().

End result: a recursive module load will still block, but now a signal
will interrupt it and fail the second module load, at which point the
first module will successfully complete loading.

Fixes: 9b9879fc03 ("modules: catch concurrent module loads, treat them as idempotent")
Cc: Russell King <linux@armlinux.org.uk>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Linus Torvalds 2024-08-09 08:33:28 -07:00
parent ee9a43b7cf
commit 2124d84db2
1 changed files with 25 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
kernel/module/main.c
View File

@ -3104,7 +3104,7 @@ static bool idempotent(struct idempotent *u, const void *cookie)
struct idempotent *existing; struct idempotent *existing;
bool first; bool first;
u->ret = 0; u->ret = -EINTR;
u->cookie = cookie; u->cookie = cookie;
init_completion(&u->complete); init_completion(&u->complete);
@ -3140,7 +3140,7 @@ static int idempotent_complete(struct idempotent *u, int ret)
hlist_for_each_entry_safe(pos, next, head, entry) { hlist_for_each_entry_safe(pos, next, head, entry) {
if (pos->cookie != cookie) if (pos->cookie != cookie)
continue; continue;
hlist_del(&pos->entry); hlist_del_init(&pos->entry);
pos->ret = ret; pos->ret = ret;
complete(&pos->complete); complete(&pos->complete);
} }
@ -3148,6 +3148,28 @@ static int idempotent_complete(struct idempotent *u, int ret)
return ret; return ret;
} }
/*
* Wait for the idempotent worker.
*
* If we get interrupted, we need to remove ourselves from the
* the idempotent list, and the completion may still come in.
*
* The 'idem_lock' protects against the race, and 'idem.ret' was
* initialized to -EINTR and is thus always the right return
* value even if the idempotent work then completes between
* the wait_for_completion and the cleanup.
*/
static int idempotent_wait_for_completion(struct idempotent *u)
{
if (wait_for_completion_interruptible(&u->complete)) {
spin_lock(&idem_lock);
if (!hlist_unhashed(&u->entry))
hlist_del(&u->entry);
spin_unlock(&idem_lock);
}
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, int flags)
{ {
struct load_info info = { }; struct load_info info = { };
@ -3191,20 +3213,8 @@ static int idempotent_init_module(struct file *f, const char __user * uargs, int
/* /*
* Somebody else won the race and is loading the module. * Somebody else won the race and is loading the module.
*
* We have to wait for it forever, since our 'idem' is
* on the stack and the list entry stays there until
* completed (but we could fix it under the idem_lock)
*
* It's also unclear what a real timeout might be,
* but we could maybe at least make this killable
* and remove the idem entry in that case?
*/ */
for (;;) { return idempotent_wait_for_completion(&idem);
if (wait_for_completion_timeout(&idem.complete, 10*HZ))
return idem.ret;
pr_warn_once("module '%pD' taking a long time to load", f);
}
} }
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags) SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)