The hex output uses a heap-based strbuf. It can be turned into a
stack-based strbuf by refactoring kmod_module_hex_to_str. Instead of
returning a C string, a supplied strbuf can be filled with hex values
in ASCII representation. Renamed to kmod_module_strbuf_pushhex.
A size of 512 is sufficient for signatures on Arch Linux and removes
heap allocations. Additional benefit is implicit strbuf_init and
strbuf_release due to DECLARE_STRBUF_WITH_STACK.
The strbuf content is never returned, so it's easy to switch to a
stack-based solution. It removes heap allocations and the need to
manually call strbuf_init and strbuf_release since these are covered
through DECLARE_STRBUF_WITH_STACK as well.
Clarify that the code does not perform error checks, for which we
regularly use checks and continue at the start, but merely checks
how to print/filter the data.
Gongjun Song [Fri, 7 Mar 2025 02:33:51 +0000 (10:33 +0800)]
testsuite: Improve fake_delete behavior
- When fake delete_module() succeeds, remove its entry from /sys/module.
- Add tests to ensure module is properly removed.
Co-developed-by: Qingqing Li <qingqing.li@intel.com> Signed-off-by: Qingqing Li <qingqing.li@intel.com> Signed-off-by: Dan He <dan.h.he@intel.com> Signed-off-by: Gongjun Song <gongjun.song@intel.com> Signed-off-by: Yuchi Chen <yuchi.chen@intel.com> Signed-off-by: Wenjie Wang <wenjie2.wang@intel.com> Link: https://github.com/kmod-project/kmod/pull/309 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Thu, 27 Feb 2025 16:47:08 +0000 (10:47 -0600)]
ci: Switch most builds to 64b
Due to a mistake on using "multilib" fixed in commit 271d8ab ("ci: Fix 32b build ignoring options") and commit 6897912 ("ci: s/multilib/x32/"), most of our builds were actually
testing 32b, which is not the most common thing.
Leave just 2 32b builds, one with Archlinux and the other with Ubuntu
and remove the FIXME about configuration.
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Thu, 27 Feb 2025 14:48:56 +0000 (08:48 -0600)]
ci: Fix 32b build ignoring options
In some configurations we pass meson_setup and x32 options, but the
options were being ignored in 32b builds. Unify the configure/build/test
steps since just the 32b configuration step that needs to be handled
differently, then make sure --native-file and the configure options
are also handled for 32b builds.
Add a 64b configuration for Archlinux and disable test on Archlinux x32
since it's failing due to packages installed and/or distro
configuration.
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Thu, 27 Feb 2025 14:38:19 +0000 (08:38 -0600)]
ci: s/multilib/x32/
It's confusing using "multilib" because it's not about adding the
"capability of building 32b", it's rather "this is really building and
testing 32b, which takes different steps.
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Thu, 27 Feb 2025 06:36:33 +0000 (00:36 -0600)]
ci: Use include to completly specify the matrix
The permutation available in github and extending it "include" or
reducing it with "exclude" are much harder than needed. It doesn't seem
we can have a mix of "properties" with the configurations. Just give up
and completly specify the matrix. Now that there's only 1 build system,
at least doing all the permutations doesn't make it too big.
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Hendrik Donner [Mon, 3 Mar 2025 15:13:01 +0000 (16:13 +0100)]
meson: Use short options for ln everywhere
Some implementations of ln (toybox, busybox) typically only support the
short option format, so use it consistently.
The short options are already used in other places and it's usually
supported on more implementations: -s and -f are POSIX and the few
ln that support --relative understand -r. Which are GNU coreutils,
the Rust uutils coreutils, toybox (-r only) and there is an old
patch for busybox (-r only). The BSDs and MacOS don't seem to
support --relative at all.
kmod: use program_invocation_short_name more often
Remove the explicit basename(argv[0]) calls because we already
rely on program_invocation_short_name even before reaching
these lines. Unifies code and slightly reduces binary size.
The write_* functions have different return statement handling. Unify
them by removing "else" if the if-block itself returns and also unify
the error handling by checking for the error case, leaving the success
return statement at the end of the function.
If no -o option is given, use stdout directly without opening
/dev/stdout manually. In a chroot environment, this could lead
to /dev/stdout creation as a regular file.
Since log_open takes a booln as argument, turn use_syslog into a
bool instead of using an int. Shrinks binary size with GCC and from
C point of view, this makes the code cleaner.
Lucas De Marchi [Mon, 6 Jan 2025 15:16:13 +0000 (09:16 -0600)]
clang-format: Add new attribute macros
Commits 44855d7 ("shared/macro: Add _alignedptr_") and d7e7c4c
("shared/macro: Add macros for more attributes") added the macros, but
forgot to add them to the clang-format configuration.
Suggested-by: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Fri, 3 Jan 2025 19:01:14 +0000 (13:01 -0600)]
testsuite: Fix build when linking with lld
When building with clang and linking with lld there is an issue with
keeping the kmod_tests sections: On ELF targets, __attribute__((used))
prevents compiler discarding, but does not affect linker --gc-sections,
according to https://lld.llvm.org/ELF/start-stop-gc.
Make sure the _start/_stop symbols are not weak and add the "retain"
attribute.
Grayson Nocera [Mon, 25 Nov 2024 20:33:02 +0000 (15:33 -0500)]
tools: specify buffer to be size PATH_MAX
Using a CodeQL query, I discovered that the destination of a `sscanf` call could overflow.
Thus, we bound the buffer size to be PATH_MAX, to ensure that it is
not larger than `modname` or `devname`.
Signed-off-by: Grayson Nocera <gnocera@purdue.edu> Suggested-by: Tobias Stoeckmann <tobias@stoeckmann.org> Link: https://github.com/kmod-project/kmod/pull/260 Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Sat, 30 Nov 2024 18:46:21 +0000 (12:46 -0600)]
ci: Fix fail due to existent build dir
The second check is a "should fail" check, but it will fail because the
build dir already exists rather than for the true reason. Use a
different dir for the configure tests and move the `rm` inside the
function.
Lucas De Marchi [Wed, 4 Dec 2024 07:19:32 +0000 (01:19 -0600)]
meson: Allow to set dlopen option for compression libraries
Add a dlopen option that allows toggling what libraries libkmod should
attempt to dlopen. If -Ddlopen=foo is passed, it means that library is
required to build, regardless of -Dfoo=*. However that library will
only be linked in if it's not set as dlopen.
Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
[ with disagreement on the need to toggle each one individually,
it'd be better to be all-or-nothing dlopen'ed ] Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/262
Lucas De Marchi [Wed, 4 Dec 2024 07:04:09 +0000 (01:04 -0600)]
build: Always define ENABLE_* macros
Define either to 0 or 1 so codebase is forced to used `#if ENABLE_*` or
similar. It's confusing to have some leaving undefined and others
defining as 0 or 1.
Lucas De Marchi [Sat, 30 Nov 2024 16:53:49 +0000 (10:53 -0600)]
libkmod/zstd: Do not re-use size_t for ret
size_t is unsigned, while the function returns a negative number.
Instead of using `ret` for the return from ZSTD_decompress(), re-use
dst_size that should be the same as we passed in for that call since
it already checks for ZSTD_CONTENTSIZE_UNKNOWN and ZSTD_CONTENTSIZE_ERROR.
Even if it's not the same, it's more correct to use that value to
assign to file->size to avoid accessing uninitialized memory.
Lucas De Marchi [Sat, 30 Nov 2024 06:30:53 +0000 (00:30 -0600)]
util: Add dlfcn helpers
Heavily based on systemd: add similar (but simplified) functions to
dlopen() a library and load specific symbols from it. This will be
useful to allow to load the compression libraries as needed. A few
differences from the systemd implementation:
1) It's allowed to link directly to the library and hence bypass the
dlopen() + dlsym()
2) The only entrypoint is dlsym_many() which is already declared with
the sentinel: it's expected callers will use an x-macro that doesn't
allow forgetting the sentinel
3) No support yet for ELF info annotation yet
Lucas De Marchi [Sun, 1 Dec 2024 00:49:00 +0000 (18:49 -0600)]
Add TAKE_PTR()
Similar to macro in systemd codebase: add a macro that documents we are
"leaking" a pointer that would otherwise be cleaned up by the cleanup
attribute.
Emil Velikov [Thu, 21 Nov 2024 17:53:47 +0000 (17:53 +0000)]
testsuite/test-weakdep: remove custom handling
Currently test-weakdep is the only test which explicitly sets the kernel
module lookup directory and the modprobe.d location. It does so by
explicitly hard-coding the full path for both, thus effectively
bypassing the path handling done in our `LD_PRELOAD` module `path.so`.
Just use `kmod_new(NULL, NULL);` which will ensure that everything is
handled relatively to `TC_ROOTFS` defined further down in the test.
Note: this technically reduces our test coverage, although kmod_new() is
not the goal of this test and should be handled separately.
Noticed while removing the TESTSUITE_ROOTFS instances from DEFINE_TEST()
Martin Wilck [Thu, 21 Nov 2024 23:02:26 +0000 (00:02 +0100)]
libkmod: hash_new(): always use a step size of at least 4
The current algorithm would choose a step size of 4 for
n_buckets == 31, but 1 for n_buckets == 32, which seems wrong.
Fix it.
Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
If buf->bytes, buf->used, and len are all 0, buf_grow() will do nothing,
and memcpy() willbe called with a NULL first argument. This will cause
an error because the function is annotated with __nonnull(1, 2).
Fixes: e62d8c7 ("strbuf: make strbuf_pushchars() a little less dumb") Signed-off-by: Martin Wilck <martin_wilck@gmx.de> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Martin Wilck [Thu, 21 Nov 2024 22:56:10 +0000 (23:56 +0100)]
testsuite: test_strbuf_pushmem: test pushing 0 bytes to an empty strbuf
This test fails, and will be fixed by the next commit.
Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Martin Wilck [Thu, 21 Nov 2024 22:45:02 +0000 (23:45 +0100)]
testsuite: test-hash: add a test for deleting a non-existing element
This test fails and will be fixed by the next commit.
Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Assume bucket->used is 1, and element 0 is deleted. We shouldn't access any
memory above (entry + 1) in this case. Likewise, if bucked->used is 2, only
one element should be shifted, etc.
Fixes: 7db0865 ("Add simple hash implementation") Signed-off-by: Martin Wilck <martin_wilck@gmx.de> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Martin Wilck [Thu, 21 Nov 2024 21:52:34 +0000 (22:52 +0100)]
testsuite: test-hash: add a test for deleting a single element
...using n_buckets = 32, which will cause a step size of 1.
This test fails, and will be fixed by the next commit.
Signed-off-by: Martin Wilck <mwilck@suse.com> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Martin Wilck [Wed, 13 Nov 2024 23:53:14 +0000 (00:53 +0100)]
man: remove duplicate option in depmod.8.scd
Fixes: c36ddb6 ("depmod: Add option to override MODULE_DIRECTORY") Signed-off-by: Martin Wilck <martin_wilck@gmx.de> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/257 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Lucas De Marchi [Tue, 19 Nov 2024 14:48:48 +0000 (08:48 -0600)]
testsuite: Remove duplicated directory
module-param-kcmdline7 was incorrectly copied inside
module-param-kcmdline{7,8}. Drop the extra dir that is not used
anywhere.
Fixes: d3a1fe67b64c ("libkmod-config: re-quote option from kernel cmdline") Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com> Reviewed-by: Emil Velikov <emil.l.velikov@gmail.com> Link: https://github.com/kmod-project/kmod/pull/254
Emil Velikov [Sat, 16 Nov 2024 15:20:42 +0000 (15:20 +0000)]
libkmod: use uint8_t for the child prefix/index
Stop implicitly casting the child prefix/index to int. It can have high
bits set thus get promoted to wildly incorrect value and cause chaos
further on.
In addition, convert the existing `unsigned char` instances to uint8_t,
which better illustrates what we're after - a fixed sized 8 bit unsigned
integer.
Emil Velikov [Sat, 16 Nov 2024 15:20:42 +0000 (15:20 +0000)]
depmod: use uint8_t for the child prefix/index
Stop implicitly casting the child prefix/index to int. It can have high
bits set thus get promoted to wildly incorrect value and cause chaos
further on.
In addition, convert the existing `unsigned char` instances to uint8_t,
which better illustrates what we're after - a fixed sized 8 bit unsigned
integer.
Use %zu for size_t, not %zd which would be for ssize_t.
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/248 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Follow up of aad7c697, which fixes the same issue in another function.
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/248 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
If newly created module in kmod_module_new could not be added to hash,
return an error. Otherwise it is possible to create multiple independent
structs with the same name, which the hash set is supposed to prevent.
An error only occurs in out of memory conditions.
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/248 Signed-off-by: Lucas De Marchi <lucas.de.marchi@gmail.com>
Emil Velikov [Fri, 15 Nov 2024 15:54:43 +0000 (15:54 +0000)]
testsuite: remove need_spawn = false support
Our test suite is a little unique in my experience in that the test can
be either a normal (fork) child or a (re)spawned one. All the other test
frameworks I have used opt for only one of the two.
I'm not entirely sure why we have both since the latter is sufficient for
all use-cases that we have. Perhaps the former was kept as
micro-optimisation?
Currently I am exploring a way to provide the results summary and the
need_spawn = false ones, are printed multiple times. At a glance I
couldn't quite find a way to fix it.
In addition I am also looking at removing/reducing the use of exit()
across the test suite. Where the two code-flows makes that process more
convoluted.
So let's remove one of the code-paths, simplify things and fix the
logging output. If needed we can re-introduce it later on.
NOTE: there's a lot going on here, because clang-format insist on
reformatting bunch of the DEFINE_TEST() instances :-\
Lucas De Marchi [Thu, 14 Nov 2024 16:33:14 +0000 (10:33 -0600)]
depmod: Fix handling relative moduledir
Make sure moduledir is always relative to the basedir and document it as
such.
Note: scdoc 1.11.3 produces a strange result when trying to render the
man page for the 2 examples. Workaround that by ending with an explicit
newline, which produces another stranger behavior (but visually correct)
of indenting the next line.
Lucas De Marchi [Thu, 14 Nov 2024 15:48:30 +0000 (09:48 -0600)]
util: Promote path_is_absolute() to header
This is a trivial function that can be used elsewhere. There's no point
in keeping the assert if we are going to crash in the very next
instruction. Rather add the relevant attribute and drop the assert.