When kmod_module_new_from_lookup() resolves to an alias, `err` will be
set to a positive value from the lookup function. Do not return a
positive value to follow the behavior when it matches a module name
and the documentation.
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Documentation says kmod_module_new_from_lookup() returns < 0 on error
and 0 otherwise. There are bugs in libkmod however making it return
a positive value in some situations, that need to be fixed. However
it's best to check for the error explicitly like is done in the rest
of the library to avoid this kind of issues.
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Since the addition of modules.aliases.bin, depmod has to open that
index multiple times and parse it over and over again:
$ sudo strace -e openat ./tools/depmod 2>&1 | grep modules.builtin.modinfo | wc -l
299
$ time sudo ./tools/depmod
real 0m7.814s
user 0m7.571s
sys 0m0.237s
Rework the logic in depmod so it does everything: open, read and parse. The
format is very straightforward and we don't need to keep it in a data structure
since we only want to add the result to a index. New output:
$ sudo strace -e openat ./tools/depmod 2>&1 | grep modules.builtin.modinfo | wc -l
1
$ time sudo ./tools/depmod
real 0m7.663s
user 0m7.516s
sys 0m0.139s
Indexes still match:
$ cmp /tmp/modules.builtin.alias.bin.new
/tmp/modules.builtin.alias.bin.old; echo $?
0
Fix: https://github.com/kmod-project/kmod/issues/11
Now that libkmod uses modules.builtin.bin again, we don't need to add
the module names in modules.builtin.alias.bin and just add the aliases.
After this change, here are the new sizes for the indexes:
Before After index
21k 6.4K modules.builtin.alias.bin
11k 11K modules.builtin.bin
The modules.builtin.alias.bin is way larger than the
modules.builtin.bin. On a normal "distro kernel":
21k modules.builtin.alias.bin
11k modules.builtin.bin
From the kernel we get both modules.builtin and modules.builtin.modinfo.
depmod generates modules.builtin.bin and modules.builtin.alias.bin
from them respectively. modules.bultin is not going away: it's not
deprecated by the new index added. So, let's just stop duplicating the
information inside modules.builtin.alias.bin and just use the other
index.
A recent bug report showed that modinfo doesn't give the signature
information for certain modules, and it turned out to happen only on
the modules that are built-in on the running kernel; then modinfo
skips the signature check, as if the target module file never exists.
The behavior is, however, inconsistent when modinfo is performed for
external modules (no matter which kernel version is) and the module
file path is explicitly given by a command-line argument, which
guarantees the presence of the module file itself.
Fixes: e7e2cb61fa ("modinfo: Show information about built-in modules")
Link: https://lore.kernel.org/linux-modules/CAKi4VAJVvY3=JdSZm-GD1hJqyCPYaYz-jBJ_REeY5BakVb6_ww@mail.gmail.com/
BugLink: https://bugzilla.opensuse.org/show_bug.cgi?id=1189537
Suggested-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
When you use pass the -v argument to modprobe we bump
the log level from the default modprobe log level of
LOG_WARNING (4) to LOG_NOTICE (5), however the library
only has avaiable to print:
#define DBG(ctx, arg...) kmod_log_cond(ctx, LOG_DEBUG, ## arg)
#define INFO(ctx, arg...) kmod_log_cond(ctx, LOG_INFO, ## arg)
#define ERR(ctx, arg...) kmod_log_cond(ctx, LOG_ERR, ## arg)
LOG_INFO (6) however is too high of a level for it to be
effective at printing anything when modprobe -v is passed.
And so the only way in which modprobe -v can trigger the
library to print a verbose message is to use ERR() but that
always prints something and we don't want that in some
situations.
We need to add a new log level macro which uses LOG_NOTICE (5)
for a "normal but significant condition" which users and developers
can use to look underneath the hood to confirm if a situation is
happening.
Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
kmod_module_new_from_name() may fail and return error value. It is
handled properly across the code, but in this particular place the
check is missing.
Signed-off-by: Yauheni Kaliuta <ykaliuta@redhat.com>
Due to wrong documentation on kmod_module_get_info() we ended up
checking for 0 as return. Check for > 0 to decided if we want to write
the index to the file, otherwise we would output a 0-sized index on
success.
Fix double free for *modinfo with non '\0' terminated wrong
modules.builtin.modinfo, which is because EOF is minus value.
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
From kmod_config_new(), when kmod_list_append() fails,
fix not list-appended kmod_config_path leak.
Signed-off-by: Seung-Woo Kim <sw0312.kim@samsung.com>
Recently in a discussion I noticed that kernel allows more chars to be
considered as space in the kernel command line. Switch to the equivalent
of isspace() instead of considering only ' '.
Add a new DEFINE_TEST_WITH_FUNC() that accepts the function
alongside the test name. This will allow us to share a single function
for different tests.
It was reported that grub mangles the kernel cmdline. It turns
acpi_cpufreq.dyndbg="file drivers/cpufreq/acpi-cpufreq.c +mpf"
into
"acpi_cpufreq.dyndbg=file drivers/cpufreq/acpi-cpufreq.c +mpf"
However, even though we could blame grub for doing that, the kernel
happily accepts and re-quotes it when the module is built-in.
So, it's better if kmod also understands it this way and does the same.
Here we basically add additional code to un-mangle it, moving the quote
in way that is acceptable to pass through init_module(). Note that the
interface [f]init_module() gives us mandates the quote to be part of the
value: the module name is not passed and the options are separated by
space.
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Tested-by: Jessica Yu <jeyu@kernel.org>
Link: https://bugzilla.suse.com/show_bug.cgi?id=1181111#c10
The handling of spaces and quotes is becoming hard to maintain. Convert
the parser into a state machine so we can check all the states. This
should make it easier to fix a corner case we have right now:
The kernel also accepts a quote before the module name instead of the
value. But this additional is left for later. This is purely an
algorithm change with no behavior change.
Tested-by: Jessica Yu <jeyu@kernel.org>
Since the output needs to be the same, regardless if the module is
compressed, change populate-modules.sh to conditionally compress the
module if that feature is enabled.
This way we can execute the tests with any build-time configuration and
it should still pass.
Suggested-by: Michal Suchánek <msuchanek@suse.de>
Reviewed-by: Michal Suchánek <msuchanek@suse.de>
Tested-by: Michal Suchánek <msuchanek@suse.de>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
For some time I've been maintaining a read-only mirror on github.
I think it's time to allow patches flowing from there besides the
mailing list: I created a new org to host the project:
https://github.com/kmod-project/kmod
Configuration in /etc should have higher prio than /run.
Given how rarely configuration in /run is used with depmod, this is
likely not to cause any problems, even if it's a change in behavior.
Bug reported by Ben Hutchings <ben@decadent.org.uk>:
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=970871
Now that the kernel provides module information for potentially
modular code that's actually built-in, it's possible to query these
built-ins with "modinfo -F". However, this doesn't work quite right:
$ modinfo -Flicense e1000e
GPL v2
$ modinfo -Flicense bitrev
name: bitrev
GPL
The Makefile helpfully warns that some tests will fail when
--sysconfdir != /etc, but there are no provisions to easily disable
those. This commit provides an escape hatch.
[ Lucas: add comment detailing the purpose of the field ]
We need a macro indirection for UNIQ to work. Otherwise it won't be
unique at all since it will just append "UNIQ" to the name:
In file included from testsuite/test-init.c:30:
testsuite/testsuite.h:142:27: error: redefinition of ‘stest_load_resourcesUNIQ’
The file is always created and unless we return an error, the temporary
file is renamed to its final destination. All other places write the
index without checking if the index is empty, so just do the same.
Reported-by: Joe Buehler <aspam@cox.net>
kmod_log_null() does not change ctx (does nothing).
Fix warnings
In file included from libkmod/libkmod-index.c:33:
libkmod/libkmod-index.c: In function ‘index_mm_open’:
libkmod/libkmod-index.c:757:6: warning: passing argument 1 of ‘kmod_log_null’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
757 | DBG(ctx, "file=%s\n", filename);
Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
idx is allocated in the beginning but it's not freed if there is
a failure after the allocation.
Change the error path: return immediately if idx allocation fails
and then free it in both success and error path at the end.
Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
The function allocates array but on building it if get_string()
fails it returns the error leaving the array allocated. The caller
does not care about it in error case either.
Free it to fix memory leak.
Signed-off-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
I changed the style of the hackargs variable in autogen.sh to multiline
because said line was becoming a bit long with the new --with-zstd arg
added.
A previous version of this patch has been running on my two Arch Linux
installations (with an accompanying mkinitcpio patch) for several months
over many kernel updates without any issues.
Any additional testing and/or patch review would of course be appreciated.
Signed-off-by: Torge Matthies <openglfreak@googlemail.com>
Lucas De Marchi
Michal Suchanek
Luis Chamberlain
Yauheni Kaliuta
Seung-Woo Kim
Dave Reisner
Marco d'Itri
Marius Bakke
Shuo Wang
Yauheni Kaliuta
Samanta Navarro
Torge Matthies