Use the variable NAME instead of "\000" directly in kmod_test_0001().
Acked-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: Tiezhu Yang yangtiezhu@loongson.cn ---
v2: - just add Acked-by tag
tools/testing/selftests/kmod/kmod.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kmod/kmod.sh b/tools/testing/selftests/kmod/kmod.sh index 3702dbc..da60c3b 100755 --- a/tools/testing/selftests/kmod/kmod.sh +++ b/tools/testing/selftests/kmod/kmod.sh @@ -341,7 +341,7 @@ kmod_test_0001_driver()
kmod_defaults_driver config_num_threads 1 - printf '\000' >"$DIR"/config_test_driver + printf $NAME >"$DIR"/config_test_driver config_trigger ${FUNCNAME[0]} config_expect_result ${FUNCNAME[0]} MODULE_NOT_FOUND } @@ -352,7 +352,7 @@ kmod_test_0001_fs()
kmod_defaults_fs config_num_threads 1 - printf '\000' >"$DIR"/config_test_fs + printf $NAME >"$DIR"/config_test_fs config_trigger ${FUNCNAME[0]} config_expect_result ${FUNCNAME[0]} -EINVAL }
There exists redundant "be an" in the comment, remove it.
Acked-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: Tiezhu Yang yangtiezhu@loongson.cn ---
v2: - just add Acked-by tag
kernel/kmod.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/kmod.c b/kernel/kmod.c index 37c3c4b..3cd075c 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -36,9 +36,8 @@ * * If you need less than 50 threads would mean we're dealing with systems * smaller than 3200 pages. This assumes you are capable of having ~13M memory, - * and this would only be an be an upper limit, after which the OOM killer - * would take effect. Systems like these are very unlikely if modules are - * enabled. + * and this would only be an upper limit, after which the OOM killer would take + * effect. Systems like these are very unlikely if modules are enabled. */ #define MAX_KMOD_CONCURRENT 50 static atomic_t kmod_concurrent_max = ATOMIC_INIT(MAX_KMOD_CONCURRENT);
If module name is empty, it is better to return directly at the beginning of request_module() without doing the needless call_modprobe() operation.
Call trace:
request_module() | | __request_module() | | call_modprobe() | | call_usermodehelper_exec() -- retval = sub_info->retval; | | call_usermodehelper_exec_work() | | call_usermodehelper_exec_sync() -- sub_info->retval = ret; | | --> call_usermodehelper_exec_async() --> do_execve() | kernel_wait4(pid, (int __user *)&ret, 0, NULL);
sub_info->retval is 256 after call kernel_wait4(), the function call_usermodehelper_exec() returns sub_info->retval which is 256, then call_modprobe() and __request_module() returns 256.
Signed-off-by: Tiezhu Yang yangtiezhu@loongson.cn ---
v2: - update the commit message to explain the detailed reason
kernel/kmod.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/kernel/kmod.c b/kernel/kmod.c index 3cd075c..5851444 100644 --- a/kernel/kmod.c +++ b/kernel/kmod.c @@ -28,6 +28,8 @@
#include <trace/events/module.h>
+#define MODULE_NOT_FOUND 256 + /* * Assuming: * @@ -144,6 +146,9 @@ int __request_module(bool wait, const char *fmt, ...) if (ret >= MODULE_NAME_LEN) return -ENAMETOOLONG;
+ if (strlen(module_name) == 0) + return MODULE_NOT_FOUND; + ret = security_kernel_module_request(module_name); if (ret) return ret;
Reset the member “test_fs” of the test configuration after a call of the function “kfree_const” to a null pointer so that a double memory release will not be performed.
Fixes: d9c6a72d6fa2 ("kmod: add test driver to stress test the module loader") Acked-by: Luis Chamberlain mcgrof@kernel.org Signed-off-by: Tiezhu Yang yangtiezhu@loongson.cn ---
v2: - update the commit message suggested by Markus Elfring - add the Fixes tag
lib/test_kmod.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/test_kmod.c b/lib/test_kmod.c index e651c37..eab5277 100644 --- a/lib/test_kmod.c +++ b/lib/test_kmod.c @@ -745,7 +745,7 @@ static int trigger_config_run_type(struct kmod_test_device *test_dev, break; case TEST_KMOD_FS_TYPE: kfree_const(config->test_fs); - config->test_driver = NULL; + config->test_fs = NULL; copied = config_copy_test_fs(config, test_str, strlen(test_str)); break;
On 04/20/2020 07:30 PM, Markus Elfring wrote:
Use the variable NAME instead of "\000" directly in kmod_test_0001().
Would this patch series have been a bit nicer together with a cover letter?
OK, thanks for your suggestion. I will resend it with a cover letter.
Thanks, Tiezhu Yang
Regards, Markus
OK, thanks for your suggestion.
Thanks for your acceptance.
I will resend it with a cover letter.
I guess that such a resend would be needed only if another version for this patch series will be published. It seems that the attention can occasionally grow also for the usage of helpful cover letters.
Regards, Markus
linux-kselftest-mirror@lists.linaro.org
-
Markus Elfring -
Tiezhu Yang