Re: [PATCH v3 3/4] kmod: Return directly if module name is empty in request_module()

[Tiezhu Yang]

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@xxxxxxxxxxx>
> 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.
>
> 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

$ cat system.c
#include <stdio.h>
#include <stdlib.h>

int main()
{
    int status = 0;

    status = system("modprobe ''");
    printf("exit status = %d\n", status);

    return status;
}

$ cat execl.c
#include <sys/wait.h>
#include <stdlib.h>
#include <unistd.h>
#include <stdio.h>

int main()
{
    pid_t pid, w;
    int status;

    pid = fork();
    if (pid == -1) {
        perror("fork");
        exit(EXIT_FAILURE);
    }

    if (pid == 0) {
        execl("/bin/sh", "sh", "-c", "modprobe aaa", (char *) 0);
    } else {
        w = waitpid(pid, &status, 0);
        if (w == -1) {
            perror("waitpid");
            exit(EXIT_FAILURE);
        }

        printf("exit status = %d\n", status);

        exit(EXIT_SUCCESS);
    }

    return 0;
}

The exit status of child process is wrote to the address of variable 
"status"
after call waitpid()in the user space that correspond with 
kernel_wait4() [1]
in the kernel space.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/exit.c#n1576