Re: module: fix failed to rmmod kernel module when it's name is too long

[Jessica Yu]

+++ Xie XiuQi [13/01/17 17:19 +0800]:
> When the name of kernel module is more than 56 chars (include 56),
> the module could be insmod successfully, but failed to rmmod.
>
> $ strace rmmod tst_1111111111222222222233333333334444444444555555555566
> ...
> open("/sys/module/tst_1111111111222222222233333333334444444444555555555566/initstate", O_RDONLY|O_CLOEXEC) = 3
> read(3, "live\n", 31)                   = 5
> read(3, "", 26)                         = 0
> close(3)                                = 0
> openat(AT_FDCWD, "/sys/module/tst_1111111111222222222233333333334444444444555555555566/holders", O_RDONLY|O_NONBLOCK|O_DIRECTORY|O_CLOEXEC) = 3
> getdents(3, /* 2 entries */, 32768)     = 48
> getdents(3, /* 0 entries */, 32768)     = 0
> close(3)                                = 0
> open("/sys/module/tst_1111111111222222222233333333334444444444555555555566/refcnt", O_RDONLY|O_CLOEXEC) = 3
> read(3, "0\n", 31)                      = 2
> read(3, "", 29)                         = 0
> close(3)                                = 0
> delete_module("tst_1111111111222222222233333333334444444444555555555566", O_RDONLY|O_NONBLOCK) = -1 ENOENT (No such file or directory)
> write(2, "rmmod: ERROR: could not remove '"..., 117rmmod: ERROR: could not remove 'tst_1111111111222222222233333333334444444444555555555566': No such file or directory
> ) = 117
> write(2, "rmmod: ERROR: could not remove m"..., 122rmmod: ERROR: could not remove module tst_1111111111222222222233333333334444444444555555555566: No such file or directory
> ) = 122
> exit_group(1)                           = ?
> +++ exited with 1 +++
>
> In this patch, we just set the last char to '\0', to make sure
> the name has the tailing '\0'.
>
> Reported-by: Zhang Liguang <zhangliguang@xxxxxxxxxx>
> Signed-off-by: Xie XiuQi <xiexiuqi@xxxxxxxxxx>
> ---
> kernel/module.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 0e54d5b..3eac266 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2928,6 +2928,7 @@ static struct module *setup_load_info(struct load_info *info, int flags)
> 	}
> 	/* This is temporary: point mod into copy of data. */
> 	mod = (void *)info->sechdrs[info->index.mod].sh_addr;
> +	mod->name[MODULE_NAME_LEN - 1] = '\0';

Does the patch actually fix the rmmod issue you're describing?

I tested it and rmmod still had issues finding the module because userspace
doesn't know its in-kernel name got truncated by one character. So kmod (rmmod)
would look for

 /sys/module/tst_1111111111222222222233333333334444444444555555555566/initstate

instead of

 /sys/module/tst_111111111122222222223333333333444444444455555555556/initstate

and subsequently fail with "module not loaded"

So, unfortunately this patch probably isn't enough fix this problem. When a
module's name is >= MODULE_NAME_LEN, it gets silently truncated in-kernel on
module load. Moreover, (1) userspace won't know about that happening and (2) the
module name found in .gnu.linkonce.this_module would also diverge from its name
in-kernel.

What might be nice to have is to have a compile-time assertion that breaks the
build if KBUILD_MODNAME exceeds MODULE_NAME_LEN, that way the error is clearly
user-visible, and we won't run into this problem in the first place. Plus
problems 1 and 2 mentioned above go away. Maybe we can have modpost insert a
BUILD_BUG_ON when KBUILD_MODNAME > MODULE_NAME_LEN?

Jessica