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

[Coly Li <colyli@suse.de>]

> 2025年1月22日 02:09，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};

OK, let me change this.

> 
>> + 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)

Yes, it might be in /usr/local/sbin, /usr/sbin/ or somewhere else which I am not
able to take it for granted on other known or unknown distributions.


> we can make in configured during compilation, simple call `which
> modprobe` should do the job.
> What do you think?

Sure, it should work. But might be more complicated, I mean lines of code in total.

Thanks for the review.

Coly Li