lkml.org 
[lkml]   [2020]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 3/4] kmod: Return directly if module name is empty in request_module()
From
Date
On 04/21/2020 10:49 PM, Jessica Yu wrote:
> +++ 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.

+Cc Al Viro <viro@zeniv.linux.org.uk>

Hi Al,

When module name is empty, __request_module() returns 256.
What do you think about this case and patch?
Thank you very much for your attention.

patch v3:
https://lore.kernel.org/patchwork/patch/1227274/

patch v4 (update the commit message):
https://lore.kernel.org/patchwork/patch/1227981/

>
> Jessica

\
 
 \ /
  Last update: 2020-04-22 10:56    [W:0.060 / U:0.428 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site