The return value of kmod_module_parse_depline and
module_get_dependencies_noref are never used, and the same is true for
n_dep in struct kmod_module.
Remove them and turn variable n in kmod_module_parse_depline into a
size_t to make sure that it never overflows.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/211 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/211 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
If functions exist which cover the exact explicitly written code, use
them instead.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/211 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
This is in sync with read_char, which uses getc_unlocked. This and
previous FILE based index adjustments improved FILE based index dump
performance by 8 %.
Emil Velikov [Thu, 24 Oct 2024 12:46:47 +0000 (13:46 +0100)]
meson: disable automatic shell completion on prefix missmatch
Currently one can choose a prefix completely different from the one
bash-completion and fish use, as per their pkg-config file.
In such cases, flag a warning and disable it. People can always manually
provide the completion directory/ies to re-enable.
$ meson setup --prefix=/tmp/example build/
...
WARNING: User provided prefix '/tmp/example' differs from bash-completion one '/usr'. Disabling completion.
...
WARNING: User provided prefix '/tmp/example' differs from fish one '/usr'. Disabling completion.
Many values are never needed, so only parse them on demand. Also keep
pointers into memory-mapped area without copying data into dynamically
sized structs, which allows nodes to be kept on stack.
Improves performance of `modprobe -c` by around 3 %.
Emil Velikov [Mon, 21 Oct 2024 13:08:59 +0000 (14:08 +0100)]
docs: annotate the deprecated API
The kmod_module_get_filtered_blacklist() was deprecated since kmod v6 in
favour of kmod_module_apply_filter().
Add the decoration so gtk-doc includes it in the generated html and also
add a designated index.
It seem that gtk-doc insists on having deprecation guards, so not it
prints an extra warning like:
warning: XXX is deprecated in the inline comments, but no deprecation
guards were found around the declaration. (See the --deprecated-guards
option for gtkdoc-scan.)
Emil Velikov [Tue, 22 Oct 2024 20:28:27 +0000 (21:28 +0100)]
meson: always pass complete path to kmod-symlink.sh
The end-user can provide either relative (to prefix) or an absolute
directory for bindir. Just fold the prefix and bindir with join_path()
which handles this correctly and pass that to kmod-symlink.sh instead of
relying on the MESON_INSTALL_DESTDIR_PREFIX environment variable.
This was previously failing due to trying to create the symlink in the
wrong location:
$ meson setup --prefix /usr --bindir /bin build-gentoo
$ DESTDIR=/tmp/install-gentoo meson install -C build-gentoo/
...
ln: failed to create symbolic link '/tmp/install-gentoo/usr//bin/depmod': No such file or directory
FAILED: install script '/home/ldmartin/p/kmod/scripts/kmod-symlink.sh /bin/depmod' failed with exit code 1.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/205
[ fix typo, add repro ] Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
On 32 bit systems it's possible to overflow the final calculation of
required memory for symbols retrieved from __ksymtab_strings.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/198 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Sun, 20 Oct 2024 12:39:02 +0000 (13:39 +0100)]
tools/depmod: use separate arrays for alias,xxxdep values
Currently, we walk the info list multiples times each time filtering all
but one key. Just create a few arrays to avoid that, saving 2-3% cycles
at the cost of extra ~500bytes per module.
Add a blank between variable declaration and function calls.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Use correct format specifiers for size_t variables.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
The contents of s and strings are identical at this point, but iterate
over the correct variable nonetheless.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
An empty section is very unlikely, so reorder code to account for it.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
The function's purpose can be merged into elf_get_section. Reduces
amount of duplicated code.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Its only caller can do the processing directly (kmod_elf_new).
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
The name of string section is not needed, so it does not have to stored.
If section name is needed, return a char pointer instead of forcing the
caller to handle offset and memory calculations.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Returned pointers are converted back to offsets in some functions. It is
more readable to turn offsets into pointers though.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
This struct name is never used. Define it just like the other ones in
kmod_elf.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
A huge module file could contain more symbols than could be represented
with an int. Use size_t instead.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
The range check should be performed in its own function for better
readability and reusability. Also, perform range checks before loops
or otherwise repeated calls by checking whole ranges instead of
single byte areas within said ranges iteratively.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/196 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Add a cheap but important check to make sure that offsets do not point
outside of memory-mapped area.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/203 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Calling qsort with NULL argument is invalid, although size 0 would
prevent anything bad from happening. Make sure that UBSAN is not
triggered.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/193 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Use shared/strbuf instead of manually re-implementing its features.
Reduces the amount of custom code in depmod, simplifies auditing,
reduces binary size, and has the nice benefit of slightly faster
runtime due to memory reusage.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/193 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
The shared/array implementation is already used within depmod, but not
for dependency output. Adding it here reduces the amount of custom code
in depmod, simplifies auditing, reduces binary size, and has the nice
benefit of slightly faster runtime due to memory reusage.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/193 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
If we have native endianess, i.e. parsing modules for the running
system, assist the compiler to note that it is really much faster to
move a word/qword etc. instead of actually running through a loop.
Reduces library instructions on x86_64 by 1.4 % and binary instructions
by 3 % with default configuration.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/187 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Compilers on x86_64 use two instructions to test value of class
variable, i.e. loading a mask and then comparing with value.
A boolean is faster, shows directly what it is about, and the struct
does not even grow.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/187 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Do not assign variables which are not even used, but merely exist for
the READV macro to work.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/187 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Since we do not want to modify the current position in file, use pread
instead of read + lseek. Removes one lseek call per module, which for
depmod on Arch Linux means 6143 calls.
Emil Velikov [Wed, 9 Oct 2024 15:26:25 +0000 (16:26 +0100)]
ci: add clang permutation
With the clang issues resolved, let's add it to the CI matrix so fewer
issues get it.
Note: Fedora 40 doesn't ship the shared sanitizer library, while older
Fedora versions did. Fedora 41 will be coming with LLVM 19, which will
have the binary (seemingly with different name :facepalm:).
Let's leave the Fedora/clang infra in and just mask it out sanitizers
for the next month or so, until the new version comes out. Then we can
re-evaluate.
Emil Velikov [Thu, 17 Oct 2024 18:17:25 +0000 (19:17 +0100)]
libkmod: _printf_format_ annotate and adjust ELFDBG modifiers
The recently added "always build ELFDBG" patch is already paying
dividends... Clang is flagging a "fmt" is not literal warning.
That's clearly wrong, although without _printf_format_ clang was
struggling to figure things out. With the attribute, it helpfully
flagged that handful of the modifiers are wrong.
Emil Velikov [Thu, 17 Oct 2024 20:30:08 +0000 (21:30 +0100)]
Swap rsync for cp --archive for module sources copying
As Tobias reported, rsync is a bit of heavyweight dependency. We
introduced it, as a replacement for the rm/cp -r previously used.
The rsync was inspired since, unlike make, meson will build all the test
binaries/artefacts even without calling "meson test".
We can go back to cp with --archive (--preserve=timestamps at least),
which will ensure we don't get stale files. To ensure the second run
doesn't copy the source folder as _subfolder_ of the dest we need to
wildcard the copy... Plus we need a proper destination folder in the
first place.
With this, we get a no-op second+ builds - be that with meson or make.
Since the explicit always-dirty state is by design, drop the meson TODO
and document the output variable.
Confirmed by comparing both the `make --debug` output and the execution
times.
Reported-by: Tobias Stoeckmann <tobias@stoeckmann.org> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/192 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
modinfo: Prevent undefined behavior with long keys
If a key is longer than INT_MAX, it is possible to trigger a signed
integer overflow. Since this overflow only occurs for formatting,
prevent it by checking if key is longer than 15 characters. If it is,
there is no need to add any more spacing anyway.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/184 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
If a module file contains parameter strings longer than INT_MAX, it is
possible to trigger an out of boundary read with memcmp. Since such a
file is very likely broken or of malicious intent, just consider it
invalid and error out.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/184 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Make sure that section is actually large enough to hold a 32 bit value.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/181 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Tue, 15 Oct 2024 19:36:50 +0000 (20:36 +0100)]
Always define and use ENABLE_ELFDBG
Convert the "if defined FOO" pre-processor checks for compiler ones "if
(FOO == 1)".
This makes things easier to reason with and ensures both code-paths are
build-tested. In case, the option is disabled DCE will kick in (assuming
you're not force disabling all optimisations) and remove the respective
code.
Emil Velikov [Tue, 15 Oct 2024 19:36:50 +0000 (20:36 +0100)]
Always define and use ENABLE_DEBUG
Convert the "if defined FOO" pre-processor checks for compiler ones "if
(FOO == 1)".
This makes things easier to reason with and ensures both code-paths are
build-tested. In case, the option is disabled DCE will kick in (assuming
you're not force disabling all optimisations) and remove the respective
code.
Emil Velikov [Tue, 15 Oct 2024 19:36:50 +0000 (20:36 +0100)]
Always define and use ENABLE_LOGGING
Convert the "if defined FOO" pre-processor checks for compiler ones "if
(FOO == 1)".
This makes things easier to reason with and ensures both code-paths are
build-tested. In case, the option is disabled DCE will kick in (assuming
you're not force disabling all optimisations) and remove the respective
code.
We try to add from environment, not from command line.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/185 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
It is always called with "__versions" as argument, so remove the
argument and rename function accordingly.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/175 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Keep changed ELF data only around as long as necessary. Otherwise it
can happen that subsequent module operations lead to unintuitive
results.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/175 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Wed, 9 Oct 2024 15:26:25 +0000 (16:26 +0100)]
testsuite: fix gcc/libasan.so load order
Currently we silence the ordering warning from libasan, via the
environment: ASAN_OPTIONS=verify_asan_link_order=0...
Instead we should be LD_PRELOAD-ing the library, since otherwise we
might end with miss-matched symbols - one coming from libasan, with the
counter part from the system library.
Plus LD_PRELOAD is the only way to make the clang sanitizers work...
That I have found. Although that's coming with a later patch.
Emil Velikov [Tue, 15 Oct 2024 12:52:07 +0000 (13:52 +0100)]
ci: disable sanitizers for ubuntu 22.04
It seems that the meson shipped with Ubuntu 22.04 is too old (buggy?) to
set the ASAN_OPTIONS and similar variables. Which means that we don't
have an easy way to detect if the sanitizers are enabled in the upcoming
test wrapper script.
The alternative is to grab newer meson, either via pip or otherwise.
Although considering we have decent coverage via the other distributions
it doesn't seem worth the hassle.
Emil Velikov [Wed, 9 Oct 2024 16:28:32 +0000 (17:28 +0100)]
ci: re-run the test verbosely when on failure
This will provide us the test log, without having to fiddle with upload
actions. Which is particularly funky since:
- v4 requires distinct names
- symbols like : (as in our container name) are not supported
Emil Velikov [Wed, 9 Oct 2024 15:26:25 +0000 (16:26 +0100)]
meson: remove no longer used HAVE_DECL___GLIBC__
Earlier commit removed the final user, drop the autotools define yet
forgot the meson one.
Fixes: 0766f541 ("build: check for __xstat declarations") Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/179 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Add missing newline for better formatting and to stay in sync with other
options.
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/182 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Fri, 11 Oct 2024 15:21:42 +0000 (16:21 +0100)]
man/insmod: minor cosmetic changes
Namely: drop the ellipsis from the man page, to align with --help.
Properly bold modprobe, without the leading space... A reminder from the
xml days.
Reference: https://lore.kernel.org/linux-modules/ZvknyLKvQeBo16n9@meinfjell.helgefjelltest.de Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/177 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Fri, 11 Oct 2024 15:19:24 +0000 (16:19 +0100)]
man: remove erroneous " in BUGS section
A silly copy/paste mistake by yours truly (-:
Fixes: ecedc689 ("man: add BUGS section with basic information") Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/177 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Wed, 9 Oct 2024 15:26:25 +0000 (16:26 +0100)]
tools/depmod: avoid casting symbol_free() function type
With clang 17, -fsanitize=function is enabled alongside any other
sanitizers. Which flags undefined behaviour, since the function type
(argument type really) is not compatible.
Emil Velikov [Wed, 9 Oct 2024 15:26:25 +0000 (16:26 +0100)]
libkmod: used unsigned type for INDEX_CHILDMAX and co
Currently the define is an signed int of 128, which we store in a char
triggering a clang warning. This is expected since the sign of char is
platform specific.
Convert the macro to unsigned and the respective variables to unsigned
char, alongside the documentation comment.
With that done, adjust the first/last checks so we don't get
tautological-compare warnings from clang.
Emil Velikov [Wed, 9 Oct 2024 15:26:25 +0000 (16:26 +0100)]
tools/insmod: use single control variable
Currently we're returning success, if any of the pre libkmod checks
fail. That's because err is our local variable, while r is the (exit)
resutlt. In practise we don't need the distinction, so just drop the
former.
Reported by clang.
Fixes: ca8f04e8 ("tools/insmod: add syslog and verbose options") Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/180
[ add blank lines, remove extra braces ] Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Thu, 3 Oct 2024 16:22:43 +0000 (17:22 +0100)]
meson: set ENABLE_{LOGGING,DEBUG} only as needed
With the introduction of the meson build, both ENABLE_LOGGING and
ENABLE_DEBUG have always been set - 1 or 0. Whereas the code relies on
them being set or unset.
Adjust the build accordingly.
Fixes: 370141c1 ("meson: introduce meson, covering libkmod.so") Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/173 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Mon, 30 Sep 2024 20:35:18 +0000 (21:35 +0100)]
shared: introduce umul{32,64}_overflow() helpers
We'll use them to implement the size_t variant.
Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/169
[ Fixup commit message for renamed functions ] Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Thu, 3 Oct 2024 16:02:45 +0000 (17:02 +0100)]
ci: directly use archlinux:multilib-devel container
There was some recent snafu which meant the container wasn't available.
With that resolved, we can drop our hack and use the correct container
directly.
Emil Velikov [Thu, 3 Oct 2024 16:02:45 +0000 (17:02 +0100)]
libkmod: silence autological compare warning
Clang will detect that the enum cannot be zero, thus triggering a
warning. Since this is an external (public API) function, the end-user
can provide any input so we want to keep the check.
Note: include the pragmas in a if defined(__clang__) guard, otherwise
we'll trigger -Wunknown_pragma which will become an error in CI and
developer builds.