Messages in this thread | | | Subject | Re: [PATCH v3 3/4] kmod: Return directly if module name is empty in request_module() | From | Tiezhu Yang <> | Date | Wed, 22 Apr 2020 16:55:34 +0800 |
| |
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
| |