Re: [PATCH] mdopen: add sbin path to env PATH when call system("modprobe md_mod")

[Mariusz Tkaczyk <mtkaczyk@kernel.org>]

On Tue, 21 Jan 2025 23:16:03 +0800
Coly Li <colyli@suse.de> wrote:

> During the boot process if mdadm is called in udev context, sbin paths
> like /sbin, /usr/sbin, /usr/local/sbin normally not defined in PATH
> env variable, calling system("modprobe md_mod") in
> create_named_array() may fail with 'sh: modprobe: command not found'
> error message.
> 
> We don't want to move modprobe binary into udev private directory, so
> setting the PATH env is a more proper method to avoid the above issue.
> 
> This patch sets PATH env variable with
> "/sbin:/usr/sbin:/usr/local/sbin" before calling system("modprobe
> md_mod"). The change only takes effect within the udev worker
> context, not seen by global udev environment.

Hi Coly,
Nice explanation, thanks!

> 
> Signed-off-by: Coly Li <colyli@suse.de>
> ---
>  mdopen.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/mdopen.c b/mdopen.c
> index 26f0c716..30cf781b 100644
> --- a/mdopen.c
> +++ b/mdopen.c
> @@ -39,6 +39,18 @@ int create_named_array(char *devnm)
>  
>  	fd = open(new_array_file, O_WRONLY);
>  	if (fd < 0 && errno == ENOENT) {
> +		char buf[PATH_MAX];
> +
> +		/*
> +		 * When called by udev worker context, path of
> modprobe
> +		 * might not be in env PATH. Set sbin paths into PATH
> +		 * env to avoid potential failure when run modprobe
> here.
> +		 */
> +		memset(buf, 0, PATH_MAX);

just:
char buf[PATH_MAX] = {0};

> +		snprintf(buf, PATH_MAX - 1, "%s:%s", getenv("PATH"),
> +			 "/sbin:/usr/sbin:/usr/local/sbin");
> +		setenv("PATH", buf, 1);

Isn't it over-complicated? Why not simply:
system("/sbin/modprobe md_mod");

If modprobe is not always in /sbin (checked on my opensuse only)
we can make in configured during compilation, simple call `which
modprobe` should do the job.
What do you think?

Thanks,
Mariusz