Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to load autofs module through autofs4 alias #9501

Closed
mbiebl opened this issue Jul 3, 2018 · 19 comments
Closed

Fails to load autofs module through autofs4 alias #9501

mbiebl opened this issue Jul 3, 2018 · 19 comments

Comments

@mbiebl
Copy link
Contributor

@mbiebl mbiebl commented Jul 3, 2018

systemd version the issue has been seen with

v239

Used distribution

Debian sid

Filed as downstream bug report https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902946

Starting with Linux 4.18, the autofs4 kernel module is renamed
to autofs, but retaining an 'autofs4' alias.

Despite the presence of the alias, systemd fails to load the
autofs module:

systemd[1]: Failed to insert module 'autofs4': No such file or directory

Opened as 'serious' since I think autofs supports critical
functionality in systemd.

@mbiebl mbiebl added the bug 🐛 label Jul 3, 2018
@boucman
Copy link
Contributor

@boucman boucman commented Jul 3, 2018

do you manage to load it manually with modprobe/insmode ?

I am asking because right now, the "fuse" module in debian testing gives me "missing symbols" errors, so it's worth double-checking the basics...

@mbiebl
Copy link
Contributor Author

@mbiebl mbiebl commented Jul 3, 2018

I don't have a 4.18 kernel to test this (yet), I'm merely forwarding the downstream bug report.

@poettering
Copy link
Member

@poettering poettering commented Jul 4, 2018

systemd loads kmods by name during the early module loading, and doesn't process aliases. I am not convinced it's really a good idea to do the full alias magic this early. I mean, the alias stuff maps names to sets of kmods iirc, it's substantially more than just a way to rename modules... urks.

Those kernel hackers with their "don't break userspace" mantra should maybe not break stuff like this so willynilly...

/cc @raven-au

@poettering
Copy link
Member

@poettering poettering commented Jul 4, 2018

Hmm, so the alias is added in this commit:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/autofs?id=a2225d931f75ddd3c39f4d0d195fad99dfd68671

The commit msg claims the alias is fine, but actually it isn't for cases like this one... If the kernel folks care that old userspace shall function correctly on new kernels they need to revert this one and a few others that are related, or find a different way out if they want to rename the kmod. /cc @torvalds

I mean, we can certainly look into working around this kernel change in userspace, and either process aliases or load autofs too, but this is not going to fix old systemd versions on new kernels...

Renaming kernel modules is problematic in general. There's lots of code in this world which checks for kernel features by accessing /sys/module/$SOMETHING. If those paths change because the kernel folks decided to rename a module then things will break. systemd for example currently uses a bunch of paths below /sys/module/:

/sys/module/vt/parameters/default_utf8
/sys/module/ipv6
/sys/module/printk/parameters/time
/sys/module/apparmor/parameters/enabled

Hence, besides the module loading itself, changing kmod names of generic kernel functionality will break stuff, all over the place.

@mbiebl
Copy link
Contributor Author

@mbiebl mbiebl commented Jul 4, 2018

Quoting Andreas Henriksson from IRC:
"
<ah[m]> Re #902946 it's likely when the following fails the fallback should be to also try kmod_module_new_from_alias (if I read the relevant code correctly on my mobile phone)...
[zwiebelbot] Debian#902946: Fails to load autofs module through autofs4 alias - https://bugs.debian.org/902946

<ah[m]> https://sources.debian.org/src/systemd/239-3/src/core/kmod-setup.c/?hl=125#L125

<ah[m]> IMHO would have been nice if kmod did that internally but it's documented not to so likely won't change: https://sources.debian.org/src/kmod/25-1/libkmod/libkmod-module.c/?hl=314#L314

<ah[m]> Or simply replace kmod_module_new_from_name with kmod_module_new_from_lookup which should dtrt and is used elsewhere in systemd source.
"

@poettering
Copy link
Member

@poettering poettering commented Jul 4, 2018

@mbiebl yeah sure, as I already said we can look into adding some fallbacks for this, but that's not going to fix old userspace on new kernels... anyway you look at it the userspace facing API of the kernel did change here...

@raven-au
Copy link

@raven-au raven-au commented Jul 4, 2018

@poettering
Copy link
Member

@poettering poettering commented Jul 4, 2018

@raven-au well, fact is kernels starting with 4.18 have problems with any released version of systemd since systemd will try to load "autofs4" and the kmod with that name ceased to exist. The MODULES_ALIAS is not honoured by systemd, as we don't follow aliases at all.

So, the rename is basically breaking all current userspace. And in my view that's a problem. And as I understand Linus' "don't break userspace" mantra this should also be considered a problem for the kernel folks.

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

@poettering
Copy link
Member

@poettering poettering commented Jul 5, 2018

Oh, wow. Linus being awful. Unlike on lkml we'd block him here for being like that. And just to make this clear: we are loading the kmod with libkmod, which isn't something the systemd folks invented but the official implementation of the kmod loader. Linus' remarks are hence entirely besides the point.

The kmod alias stuff is usually used to match kmods to pci or usb vid/pid and similar things, not as a hack to rename specific modules. We just wanted to load four specific general purpose kmods which some distros build as module and hence we reference them by name, in particular has the alias stuff allows non-unique mappings which is not of interest for this.

Anyway, if this is the level of conversation the kernel folks want then don't expect any further comment here from me on this topic. I am out on this one.

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

@raven-au
Copy link

@raven-au raven-au commented Jul 5, 2018

torvalds added a commit to torvalds/linux that referenced this issue Jul 5, 2018
It turns out that systemd has a bug: it wants to load the autofs module
early because of some initialization ordering with udev, and it doesn't
do that correctly.  Everywhere else it does the proper "look up module
name" that does the proper alias resolution, but in that early code, it
just uses a hardcoded "autofs4" for the module name.

The result of that is that as of commit a2225d9 ("autofs: remove
left-over autofs4 stubs"), you get

    systemd[1]: Failed to insert module 'autofs4': No such file or directory

in the system logs, and a lack of module loading.  All this despite the
fact that we had very clearly marked 'autofs4' as an alias for this
module.

What's so ridiculous about this is that literally everything else does
the module alias handling correctly, including really old versions of
systemd (that just used 'modprobe' to do this), and even all the other
systemd module loading code.

Only that special systemd early module load code is broken, hardcoding
the module names for not just 'autofs4', but also "ipv6", "unix",
"ip_tables" and "virtio_rng".  Very annoying.

Instead of creating an _additional_ separate compatibility 'autofs4'
module, just rely on the fact that everybody else gets this right, and
just call the module 'autofs4' for compatibility reasons, with 'autofs'
as the alias name.

That will allow the systemd people to fix their bugs, adding the proper
alias handling, and maybe even fix the name of the module to be just
"autofs" (so that they can _test_ the alias handling).  And eventually,
we can revert this silly compatibility hack.

See also

    systemd/systemd#9501
    https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=902946

for the systemd bug reports upstream and in the Debian bug tracker
respectively.

Fixes: a2225d9 ("autofs: remove left-over autofs4 stubs")
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Reported-by: Michael Biebl <biebl@debian.org>
Cc: Ian Kent <raven@themaw.net>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
@raven-au
Copy link

@raven-au raven-au commented Jul 6, 2018

keszybz added a commit to keszybz/systemd that referenced this issue Jul 6, 2018
This allows aliases to be used for the basic modules we load from pid1 before
udev is started. In systemd#9501 the kernel renamed autofs4 to autofs, with "autofs4"
as alias, but we wouldn't load the module, because we didn't follow aliases.
The kernel change was reverted, but it's probably better to support aliases.
@raven-au
Copy link

@raven-au raven-au commented Jul 7, 2018

keszybz added a commit to keszybz/systemd that referenced this issue Jul 16, 2018
This allows aliases to be used for the basic modules we load from pid1 before
udev is started. In systemd#9501 the kernel renamed autofs4 to autofs, with "autofs4"
as alias, but we wouldn't load the module, because we didn't follow aliases.
The kernel change was reverted, but it's probably better to support aliases.
keszybz added a commit to keszybz/systemd that referenced this issue Jul 19, 2018
This allows aliases to be used for the basic modules we load from pid1 before
udev is started. In systemd#9501 the kernel renamed autofs4 to autofs, with "autofs4"
as alias, but we wouldn't load the module, because we didn't follow aliases.
The kernel change was reverted, but it's probably better to support aliases.
keszybz added a commit to keszybz/systemd that referenced this issue Oct 28, 2018
This allows aliases to be used for the basic modules we load from pid1 before
udev is started. In systemd#9501 the kernel renamed autofs4 to autofs, with "autofs4"
as alias, but we wouldn't load the module, because we didn't follow aliases.
The kernel change was reverted, but it's probably better to support aliases.

(cherry picked from commit 81d7c69)
@mbiebl
Copy link
Contributor Author

@mbiebl mbiebl commented Oct 29, 2018

I think this issue is fixed.
For one, the kernel re-added the workaround and @keszybz also addressed this in commit 81d7c69

@mbiebl mbiebl closed this Oct 29, 2018
fpletz pushed a commit to NixOS/systemd that referenced this issue Oct 31, 2018
This allows aliases to be used for the basic modules we load from pid1 before
udev is started. In systemd#9501 the kernel renamed autofs4 to autofs, with "autofs4"
as alias, but we wouldn't load the module, because we didn't follow aliases.
The kernel change was reverted, but it's probably better to support aliases.

(cherry picked from commit 81d7c69)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants