Skip to content

Commit ac2263b

Browse files
committed
Revert "module: error out early on concurrent load of the same module file"
This reverts commit 9828ed3. Sadly, it does seem to cause failures to load modules. Johan Hovold reports: "This change breaks module loading during boot on the Lenovo Thinkpad X13s (aarch64). Specifically it results in indefinite probe deferral of the display and USB (ethernet) which makes it a pain to debug. Typing in the dark to acquire some logs reveals that other modules are missing as well" Since this was applied late as a "let's try this", I'm reverting it asap, and we can try to figure out what goes wrong later. The excessive parallel module loading problem is annoying, but not noticeable in normal situations, and this was only meant as an optimistic workaround for a user-space bug. One possible solution may be to do the optimistic exclusive open first, and then use a lock to serialize loading if that fails. Reported-by: Johan Hovold <[email protected]> Link: https://lore.kernel.org/lkml/[email protected]/ Signed-off-by: Linus Torvalds <[email protected]>
1 parent e338142 commit ac2263b

File tree

2 files changed

+15
-49
lines changed

2 files changed

+15
-49
lines changed

include/linux/fs.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2566,12 +2566,6 @@ static inline int deny_write_access(struct file *file)
25662566
struct inode *inode = file_inode(file);
25672567
return atomic_dec_unless_positive(&inode->i_writecount) ? 0 : -ETXTBSY;
25682568
}
2569-
static inline int exclusive_deny_write_access(struct file *file)
2570-
{
2571-
int old = 0;
2572-
struct inode *inode = file_inode(file);
2573-
return atomic_try_cmpxchg(&inode->i_writecount, &old, -1) ? 0 : -ETXTBSY;
2574-
}
25752569
static inline void put_write_access(struct inode * inode)
25762570
{
25772571
atomic_dec(&inode->i_writecount);

kernel/module/main.c

Lines changed: 15 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3057,13 +3057,25 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
30573057
return load_module(&info, uargs, 0);
30583058
}
30593059

3060-
static int file_init_module(struct file *file, const char __user * uargs, int flags)
3060+
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
30613061
{
30623062
struct load_info info = { };
30633063
void *buf = NULL;
30643064
int len;
3065+
int err;
3066+
3067+
err = may_init_module();
3068+
if (err)
3069+
return err;
3070+
3071+
pr_debug("finit_module: fd=%d, uargs=%p, flags=%i\n", fd, uargs, flags);
30653072

3066-
len = kernel_read_file(file, 0, &buf, INT_MAX, NULL,
3073+
if (flags & ~(MODULE_INIT_IGNORE_MODVERSIONS
3074+
|MODULE_INIT_IGNORE_VERMAGIC
3075+
|MODULE_INIT_COMPRESSED_FILE))
3076+
return -EINVAL;
3077+
3078+
len = kernel_read_file_from_fd(fd, 0, &buf, INT_MAX, NULL,
30673079
READING_MODULE);
30683080
if (len < 0) {
30693081
mod_stat_inc(&failed_kreads);
@@ -3072,7 +3084,7 @@ static int file_init_module(struct file *file, const char __user * uargs, int fl
30723084
}
30733085

30743086
if (flags & MODULE_INIT_COMPRESSED_FILE) {
3075-
int err = module_decompress(&info, buf, len);
3087+
err = module_decompress(&info, buf, len);
30763088
vfree(buf); /* compressed data is no longer needed */
30773089
if (err) {
30783090
mod_stat_inc(&failed_decompress);
@@ -3087,46 +3099,6 @@ static int file_init_module(struct file *file, const char __user * uargs, int fl
30873099
return load_module(&info, uargs, flags);
30883100
}
30893101

3090-
/*
3091-
* kernel_read_file() will already deny write access, but module
3092-
* loading wants _exclusive_ access to the file, so we do that
3093-
* here, along with basic sanity checks.
3094-
*/
3095-
static int prepare_file_for_module_load(struct file *file)
3096-
{
3097-
if (!file || !(file->f_mode & FMODE_READ))
3098-
return -EBADF;
3099-
if (!S_ISREG(file_inode(file)->i_mode))
3100-
return -EINVAL;
3101-
return exclusive_deny_write_access(file);
3102-
}
3103-
3104-
SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
3105-
{
3106-
struct fd f;
3107-
int err;
3108-
3109-
err = may_init_module();
3110-
if (err)
3111-
return err;
3112-
3113-
pr_debug("finit_module: fd=%d, uargs=%p, flags=%i\n", fd, uargs, flags);
3114-
3115-
if (flags & ~(MODULE_INIT_IGNORE_MODVERSIONS
3116-
|MODULE_INIT_IGNORE_VERMAGIC
3117-
|MODULE_INIT_COMPRESSED_FILE))
3118-
return -EINVAL;
3119-
3120-
f = fdget(fd);
3121-
err = prepare_file_for_module_load(f.file);
3122-
if (!err) {
3123-
err = file_init_module(f.file, uargs, flags);
3124-
allow_write_access(f.file);
3125-
}
3126-
fdput(f);
3127-
return err;
3128-
}
3129-
31303102
/* Keep in sync with MODULE_FLAGS_BUF_SIZE !!! */
31313103
char *module_flags(struct module *mod, char *buf, bool show_state)
31323104
{

0 commit comments

Comments
 (0)