GNU bug report logs - #30604
[PATCH 0/4] Load Linux module only when supported hardware is present.

Previous Next

Full log

Message #320 received at 30604 <at> debbugs.gnu.org (full text, mbox):

From: ludo <at> gnu.org (Ludovic Courtès)
To: Danny Milosavljevic <dannym <at> scratchpost.org>
Cc: 30604 <at> debbugs.gnu.org
Subject: Re: [bug#30604] [PATCH v10 5/6] linux-initrd: Provide our own
 'modprobe' program.
Date: Mon, 12 Mar 2018 23:14:59 +0100

Howdy!

Danny Milosavljevic <dannym <at> scratchpost.org> skribis:

> On Mon, 12 Mar 2018 13:39:17 +0100
> Ludovic Courtès <ludo <at> gnu.org> 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?

You’re right, I wondered what would happen.  I’ve restored the
/sbin/modprobe symlink in v11.

>> +          (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.

Indeed, done in v11.

>>+(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!

I didn’t mean to leave it, but after all, it’s pretty useful.  :-)

> 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:

Sure.

>> +          (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.

We’re fine because btrfs, 9p, overlay, etc. all have an “fs-btrfs”,
“fs-9p”, etc. alias, which shows up in “modules.alias”.  No need for
“modules.devname” AFAICS.

> (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?)

The dependency of a builtin is necessarily a builtin, and the kernel
won’t invoke modprobe for a builtin, will it?  IOW, I think there’s no
problem here.  :-)

>> +         ;; We may need to lazy-load modules.  The initrd installs a
>
> Nitpick: "kernel modules" (to clarify).

Fixed.

Thanks for the insightful review!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.