From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752080AbdASD12 (ORCPT ); Wed, 18 Jan 2017 22:27:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38502 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751952AbdASD11 (ORCPT ); Wed, 18 Jan 2017 22:27:27 -0500 Date: Wed, 18 Jan 2017 22:26:59 -0500 From: Jessica Yu To: Xie XiuQi Cc: rusty@rustcorp.com.au, linux-kernel@vger.kernel.org, zhangliguang@huawei.com Subject: Re: module: fix failed to rmmod kernel module when it's name is too long Message-ID: <20170119032659.GA17733@packer-debian-8-amd64.digitalocean.com> References: <1484299143-157054-1-git-send-email-xiexiuqi@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <1484299143-157054-1-git-send-email-xiexiuqi@huawei.com> X-OS: Linux eisen.io 3.16.0-4-amd64 x86_64 User-Agent: Mutt/1.5.23 (2014-03-12) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 19 Jan 2017 03:27:00 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org +++ 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 >Signed-off-by: Xie XiuQi >--- > 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