From: Jessica Yu <jeyu@kernel.org>
To: Tiezhu Yang <yangtiezhu@loongson.cn>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
Shuah Khan <shuah@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org,
Markus Elfring <Markus.Elfring@web.de>,
Xuefeng Li <lixuefeng@loongson.cn>
Subject: Re: [PATCH v3 3/4] kmod: Return directly if module name is empty in request_module()
Date: Tue, 21 Apr 2020 16:49:32 +0200 [thread overview]
Message-ID: <20200421144931.GA20103@linux-8ccs> (raw)
In-Reply-To: <675147f7-2762-c574-4c3d-de6b25a5a44a@loongson.cn>
+++ Tiezhu Yang [21/04/20 11:07 +0800]:
>On 04/21/2020 02:19 AM, Luis Chamberlain wrote:
>>On Mon, Apr 20, 2020 at 08:33:54PM +0800, Tiezhu Yang wrote:
>>>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>
>>Thanks for looking into this. I still cannot find where
>>userspace it returns 256. Can you? If I run modprobe without
>>an argument I see 1 returned.
>>
>>At least kmod [0] has a series of cmd helper structs, the one for modprobe
>>seems to be kmod_cmd_compat_modprobe, and I can see -1 returned which
>>can be converted to 255. It can also return EXIT_FAILURE or EXIT_SUCCESS
>>and /usr/include/stdlib.h defines these as 1 and 0 respectively.
I'm also seeing modprobe return 1 as exit status when I run it without
arguments. I don't think the 256 value is coming from modprobe though,
see below -
>>https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/
>>
>> Luis
>
>Here is my understanding:
>
>When build and execute the following application, we can see the exit
>status is 256.
>
>$ ./system
>modprobe: FATAL: Module not found in directory
>/lib/modules/4.18.0-147.5.1.el8_1.x86_64
>exit status = 256
>
>$ ./execl
>modprobe: FATAL: Module not found in directory
>/lib/modules/4.18.0-147.5.1.el8_1.x86_64
>exit status = 256
I am going to guess this has something to do with how system() and
waitpid() (and the wait family of syscalls in general) encode the exit
status in their return values. According to their man pages, you need
to use the appropriate WIF* macros to get the actual exit code of the
child process.
From system(3):
the return value is a "wait status" that can be examined using the
macros described in waitpid(2). (i.e., WIFEXITED(),
WEXITSTATUS(), and so on)
From waitpid(2):
If wstatus is not NULL, wait() and waitpid() store status
information in the int to which it points. This integer can be
inspected with the following macros (which take the integer
itself as an argument, not a pointer to it, as is done in wait()
and waitpid()!):
WEXITSTATUS(wstatus)
returns the exit status of the child. This consists of
the least significant 8 bits of the status argument that
the child specified in a call to exit(3) or _exit(2) or
as the argument for a return statement in main(). This
macro should be employed only if WIFEXITED returned
true.
In your test code, you are reading &status directly. To obtain the
exit status, you need to use WEXITSTATUS(status), or right shift the
value by 8 bits. That gives you 1, which was the original exit code
given by modprobe. That's why you see an exit code of 1 when running
modprobe directly and you see 256 when using system() and waitpid()
and don't use the WIF* macros.
As for why __request_module() returns 256, I am guessing this would
come from kernel_wait4(), but I did not dive into the call path to
verify this yet.
Jessica
next prev parent reply other threads:[~2020-04-21 14:49 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-20 12:33 [PATCH v3 0/4] Fix some issues about kmod Tiezhu Yang
2020-04-20 12:33 ` [PATCH v3 1/4] selftests: kmod: Use variable NAME in kmod_test_0001() Tiezhu Yang
2020-04-20 12:33 ` [PATCH v3 2/4] kmod: Remove redundant "be an" in the comment Tiezhu Yang
2020-04-20 12:33 ` [PATCH v3 3/4] kmod: Return directly if module name is empty in request_module() Tiezhu Yang
2020-04-20 18:19 ` Luis Chamberlain
2020-04-21 3:07 ` Tiezhu Yang
2020-04-21 14:49 ` Jessica Yu [this message]
2020-04-22 8:31 ` Luis Chamberlain
2020-04-22 8:55 ` Tiezhu Yang
2020-04-22 9:01 ` Luis Chamberlain
2020-04-20 12:33 ` [PATCH v3 4/4] test_kmod: Avoid potential double free in trigger_config_run_type() Tiezhu Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200421144931.GA20103@linux-8ccs \
--to=jeyu@kernel.org \
--cc=Markus.Elfring@web.de \
--cc=akpm@linux-foundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=lixuefeng@loongson.cn \
--cc=mcgrof@kernel.org \
--cc=shuah@kernel.org \
--cc=yangtiezhu@loongson.cn \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).