Subject: Re: [bug#30604] [PATCH v10 5/6] linux-initrd: Provide our own 'modprobe' program.

From: Danny Milosavljevic <[email-masked]>
Date: Mon, 12 Mar 2018 21:09:36 +0100

Hi Ludo,

On Mon, 12 Mar 2018 13:39:17 +0100
Ludovic Courtès <[email-masked]> wrote:

> +       (when modprobe
> +         ;; Tell the kernel to invoke MODPROBE.
> +         (call-with-output-file "/proc/sys/kernel/modprobe"
> +           (lambda (port)
> +             (display modprobe port))))
> +
>         (display "loading kernel modules...\n")

There's a race because Linux could just start loading modules
immediately before boot-system even starts / executed far enough.

That was why earlier I kept /sbin/modprobe as the name (it's the
default Linux uses) and made sure that the file /sbin/modprobe is already
in the initrd (as opposed to created by boot-system).

The race didn't happen to me yet (and I doubt that Linux has modules
fine-grained enough that that's a problem in practise) - but I would prefer
it not to be possible.

WDYT?

> +          (define aliases
> +            ;; The list of aliases we are asked to load.
> +            (filter-map (match-lambda
> +                          (('argument . alias) alias)
> +                          (_ #f))
> +                        options))

Without "-a", it's not supposed to be a list of aliases.  It's supposed to be
one alias and the remainder are parameters supposed to be passed to that
kernel module.

(I didn't know that before).

When Linux invokes /sbin/modprobe, it doesn't pass "-a" but it doesn't
pass any module parameters either - it just passes one module alias.

For clarity and in order that we don't have weird problems later (trying to
load a module named "verbose=1" or something :) ), let's just accept one alias
for now.

>+(define* (modprobe-program linux-module-directory #:key
>+                           (guile %guile-static-stripped))
[...]
> +            (call-with-output-file "/dev/kmsg"
> +              (lambda (port)
> +                (format port "modprobe[~a]: aliases ~s; modules ~s; args ~s~%"
> +                        (getpid) aliases modules (program-arguments))))

Hmm, logging is nice!

But with /dev/kmsg one write(2) syscall needs to pass the entire message.

What do you think about additionally calling setvbuf with 1024 ?  Because:

kernel/printk/printk.c: #define LOG_LINE_MAX         (1024 - PREFIX_MAX)

> +            (when (< (length modules) (length aliases))
> +              (error "alias resolution failed" aliases))

The original modprobe loads as many modules as it can and possibly sets an
(always the same) error flag (and continues loading the other modules).

Then modprobe's exit status is that error flag.

This is moot because we shouldn't accept more than one alias - see above.

> +          (write-module-alias-database #$output))))

One of the devnames created statically is the one for btrfs, so not writing or
using devnames is not going to end well.

If it did work without it I'd be even more on guard :)

(I'd also copy the modules.builtin (from Linux).
 Also, what happens if we load a module which has as dependency a builtin?
 Will we try to load the builtin as a .ko file and fail the entire thing?)

> +         ;; We may need to lazy-load modules.  The initrd installs a

Nitpick: "kernel modules" (to clarify).