From 9e8a4d8b5f9ae63b56c207d5d71ee3440d4fd5cf Mon Sep 17 00:00:00 2001 From: Douglas Raillard <douglas.raillard@arm.com> Date: Wed, 16 Aug 2023 12:24:39 +0100 Subject: [PATCH 1/2] lisa._kmod: Fix pre-existing .ko loading FIX Refactor DynamicKmod.install() to abstract over how the kernel module makes its way to the target and fix loading of an existing lisa.ko module. Also fix module checksum so that Python compuation matches what the Makefile computes. --- lisa/_assets/kmodules/lisa/Makefile | 24 ++-- lisa/_assets/kmodules/lisa/main.c | 4 +- lisa/_assets/kmodules/lisa/sched_helpers.h | 2 +- lisa/_kmod.py | 155 ++++++++++++++------- 4 files changed, 119 insertions(+), 66 deletions(-) diff --git a/lisa/_assets/kmodules/lisa/Makefile b/lisa/_assets/kmodules/lisa/Makefile index b10f50bd7..ec765c253 100644 --- a/lisa/_assets/kmodules/lisa/Makefile +++ b/lisa/_assets/kmodules/lisa/Makefile @@ -45,9 +45,13 @@ $(LISA_KMOD_NAME)-y := main.o tp.o wq.o features.o pixel6.o ccflags-y = "-I$(MODULE_SRC)" -std=gnu11 -fno-stack-protector -Wno-declaration-after-statement FEATURES_LDS := features.lds +GENERATED = $(MODULE_OBJ)/generated -SYMBOL_NAMESPACES_H = $(MODULE_OBJ)/symbol_namespaces.h -MODULE_VERSION_H = $(MODULE_OBJ)/module_version.h +$(GENERATED): + mkdir -p "$@" + +SYMBOL_NAMESPACES_H = $(GENERATED)/symbol_namespaces.h +MODULE_VERSION_H = $(GENERATED)/module_version.h # in-tree build ifneq ($(srctree),.) @@ -58,11 +62,11 @@ ldflags-y += -T $(srctree)/$(obj)/$(FEATURES_LDS) # out-of-tree build else -VMLINUX_H = $(MODULE_OBJ)/vmlinux.h +VMLINUX_H = $(GENERATED)/vmlinux.h ldflags-y += -T $(M)/$(FEATURES_LDS) -clean-files := vmlinux.h +clean-files := $(VMLINUX_H) $(SYMBOL_NAMESPACES_H) $(MODULE_VERSION_H) VMLINUX_TXT = $(MODULE_SRC)/private_types.txt @@ -78,7 +82,7 @@ endif VMLINUX_H_TYPE_PREFIX=KERNEL_PRIVATE_ -$(VMLINUX_H): $(VMLINUX_TXT) $(VMLINUX) +$(VMLINUX_H): $(GENERATED) $(VMLINUX_TXT) $(VMLINUX) # Some options are not upstream (yet) but they have all be published on the # dwarves mailing list # @@ -126,14 +130,14 @@ endif # kernel so it would be way to slow. # There does not seem to be any other source for the info in e.g. sysfs or # procfs, so we rely on that hack for now. -$(SYMBOL_NAMESPACES_H): +$(SYMBOL_NAMESPACES_H): $(GENERATED) find "$(KERNEL_SRC)" '(' -name '*.c' -o -name '*.h' ')' -print0 | xargs -0 sed -n 's/MODULE_IMPORT_NS([^;]*;/\0/p' | sort -u > $@ echo "MODULE_IMPORT_NS(VFS_internal_I_am_really_a_filesystem_and_am_NOT_a_driver);" >> $@ -$(MODULE_VERSION_H): - echo -n '#ifndef _MODULE_VERSION_H\n#define _MODULE_VERSION_H\n#define LISA_MODULE_VERSION "' > $@ - cat $$(find $(MODULE_SRC)/ -name '*.*' | sort) | sha1sum | awk '{printf "%s\"\n", $$1}' >> $@ - echo '#endif' >> $@ +$(MODULE_VERSION_H): $(GENERATED) + printf "#ifndef _MODULE_VERSION_H\n#define _MODULE_VERSION_H\n#define LISA_MODULE_VERSION \"" > $@ + export LC_ALL=C && cd $(MODULE_SRC) && sha1sum -- *.c *.h | sort | sha1sum | cut -d' ' -f1 | tr -d '\n' >> $@ + printf "\"\n#endif\n" >> $@ # Make all object files depend on the generated sources $(addprefix $(MODULE_OBJ)/,$($(LISA_KMOD_NAME)-y)): $(VMLINUX_H) $(SYMBOL_NAMESPACES_H) $(MODULE_VERSION_H) diff --git a/lisa/_assets/kmodules/lisa/main.c b/lisa/_assets/kmodules/lisa/main.c index 5b2dab81e..a3d1b3942 100644 --- a/lisa/_assets/kmodules/lisa/main.c +++ b/lisa/_assets/kmodules/lisa/main.c @@ -3,11 +3,11 @@ #include "main.h" #include "features.h" -#include "module_version.h" +#include "generated/module_version.h" /* Import all the symbol namespaces that appear to be defined in the kernel * sources so that we won't trigger any warning */ -#include "symbol_namespaces.h" +#include "generated/symbol_namespaces.h" static char* version = LISA_MODULE_VERSION; module_param(version, charp, 0); diff --git a/lisa/_assets/kmodules/lisa/sched_helpers.h b/lisa/_assets/kmodules/lisa/sched_helpers.h index 23099eb5b..8dee6ae91 100644 --- a/lisa/_assets/kmodules/lisa/sched_helpers.h +++ b/lisa/_assets/kmodules/lisa/sched_helpers.h @@ -13,7 +13,7 @@ #else -#include "vmlinux.h" +#include "generated/vmlinux.h" #ifdef CONFIG_FAIR_GROUP_SCHED static inline struct rq *rq_of(struct cfs_rq *cfs_rq) diff --git a/lisa/_kmod.py b/lisa/_kmod.py index 57361583a..ca8851405 100644 --- a/lisa/_kmod.py +++ b/lisa/_kmod.py @@ -147,6 +147,14 @@ from lisa._unshare import ensure_root import lisa._git as git from lisa.conf import SimpleMultiSrcConf, TopLevelKeyDesc, LevelKeyDesc, KeyDesc + +class KmodVersionError(Exception): + """ + Raised when the kernel module is not found with the expected version. + """ + pass + + _ALPINE_ROOTFS_URL = 'https://dl-cdn.alpinelinux.org/alpine/v{minor}/releases/{arch}/alpine-minirootfs-{version}-{arch}.tar.gz' def _abi_to_kernel_arch(abi): @@ -1727,18 +1735,27 @@ class KmodSrc(Loggable): } def _checksum(self, sources_only=False): - m = hashlib.sha1() - sources = { - name: content - for name, content - in self.src.items() - if '.' in name - } if sources_only else self.src - for name, content in sorted(sources.items()): - if not sources_only: - m.update(name.encode('utf-8')) + sources = self.code_files if sources_only else self.src + + def checksum(content): + m = hashlib.sha1() + content = content if isinstance(content, bytes) else content.encode('utf-8') m.update(content) - return m.hexdigest() + return m.hexdigest() + + content = sorted( + (checksum(content), name) + for name, content in sources.items() + ) + + # Recreate the output of sha1sum over multiple files, and checksum + # that. + return checksum( + '\n'.join( + f'{csum} {name}' + for csum, name in content + ) + '\n' + ) @property @memoized @@ -1750,7 +1767,7 @@ class KmodSrc(Loggable): @property @memoized - def checksum_sources(self): + def sources_checksum(self): """ Checksum of the module's sources. """ @@ -2095,14 +2112,34 @@ class DynamicKmod(Loggable): string following the ``module_param_array()`` kernel API syntax. :type kmod_params: dict(str, object) or None """ - # Avoid circular import - from lisa.trace import DmesgCollector + target = self.target def target_mktemp(): return target.execute( f'mktemp -p {quote(target.working_directory)}' ).strip() + @contextlib.contextmanager + def kmod_cm(): + content = self._compile() + with tempfile.NamedTemporaryFile('wb', suffix='.ko') as f: + f.write(content) + f.flush() + + target_temp = Path(target_mktemp()) + host_temp = Path(f.name) + try: + target.push(str(host_temp), str(target_temp)) + yield target_temp + finally: + target.remove(str(target_temp)) + + return self._install(kmod_cm(), kmod_params=kmod_params) + + def _install(self, kmod_cm, kmod_params): + # Avoid circular import + from lisa.trace import DmesgCollector + def make_str(x): if isinstance(x, str): return x @@ -2127,7 +2164,6 @@ class DynamicKmod(Loggable): logger = self.logger target = self.target - content = self._compile() kmod_params = kmod_params or {} params = ' '.join( @@ -2138,28 +2174,31 @@ class DynamicKmod(Loggable): ) ) - with tempfile.NamedTemporaryFile('wb', suffix='.ko') as f, tempfile.NamedTemporaryFile() as dmesg_out: + try: + self.uninstall() + except Exception: + pass + + with kmod_cm as ko_path, tempfile.NamedTemporaryFile() as dmesg_out: dmesg_coll = ignore_exceps( Exception, DmesgCollector(target, output_path=dmesg_out.name), lambda when, cm, excep: logger.error(f'Encounted exceptions while {when}ing dmesg collector: {excep}') ) - f.write(content) - f.flush() - - temp_ko = target_mktemp() try: - target.push(f.name, temp_ko) with dmesg_coll as dmesg_coll: - target.execute(f'{quote(target.busybox)} insmod {quote(temp_ko)} {params}', as_root=True) - except Exception: + target.execute(f'{quote(target.busybox)} insmod {quote(str(ko_path))} {params}', as_root=True) + + except Exception as e: log_dmesg(dmesg_coll, logger.error) - raise + + if isinstance(e, TargetStableCalledProcessError) and e.returncode == errno.EPROTO: + raise KmodVersionError('In-tree module version does not match what LISA expects.') + else: + raise else: log_dmesg(dmesg_coll, logger.debug) - finally: - target.remove(temp_ko) def uninstall(self): """ @@ -2314,40 +2353,50 @@ class LISAFtraceDynamicKmod(FtraceDynamicKmod): ) def install(self, kmod_params=None): - try: - self.uninstall() - except Exception: - pass - busybox = quote(self.target.busybox) - modules_path_base = '/lib/modules' - modules_version = self.target.kernel_version.release + target = self.target + logger = self.logger + busybox = quote(target.busybox) + + def guess_kmod_path(): + modules_path_base = '/lib/modules' + modules_version = target.kernel_version.release + + if target.os == 'android': + modules_path_base = f'/vendor_dlkm{modules_path_base}' + # Hack for GKI modules where the path might not match the kernel's + # uname -r + try: + modules_version = Path(target.execute( + f"{busybox} find {modules_path_base} -maxdepth 1 -mindepth 1 | {busybox} head -1" + ).strip()).name + except TargetStableCalledProcessError: + pass + + base_path = f"{modules_path_base}/{modules_version}" + return (base_path, f"{self.src.mod_name}.ko") - if self.target.os == 'android': - modules_path_base = f'/vendor_dlkm{modules_path_base}' - try: - modules_version = Path(self.target.execute( - f"{busybox} find {modules_path_base} -maxdepth 1 -mindepth 1 | {busybox} head -1" - ).strip()).name - except TargetStableCalledProcessError: - pass - modules_path = f"{modules_path_base}/{modules_version}" - lisa_module_filename = f"{self.src.mod_name}.ko" + kmod_params = kmod_params or {} + kmod_params['version'] = self.src.sources_checksum + base_path, kmod_filename = guess_kmod_path() try: - lisa_module_path = self.target.execute( - f"{busybox} find {modules_path} -name {quote(lisa_module_filename)}" + kmod_path = target.execute( + f"{busybox} find {base_path} -name {quote(kmod_filename)}" ).strip() - self.target.execute( - f"{busybox} insmod {lisa_module_path} version={self.src.checksum_sources}" - ) - except TargetStableCalledProcessError as e: - if e.returncode == errno.EPROTO: - raise ValueError('In-tree module version does not match what Lisa expects.') - super().install(kmod_params=kmod_params) + @contextlib.contextmanager + def kmod_cm(): + yield kmod_path + + ret = self._install(kmod_cm(), kmod_params=kmod_params) + except (TargetStableCalledProcessError, KmodVersionError) as e: + logger.debug(f'Pre-existing {kmod_filename} was not found or does not have the expected version: {e}') + ret = super().install(kmod_params=kmod_params) else: - self.logger.debug(f'Loaded module {self.src.mod_name} from kernel modules directory') + logger.debug(f'Loaded "{self.src.mod_name}" module from pre-installed location: {kmod_path}') + + return ret # vim :set tabstop=4 shiftwidth=4 expandtab textwidth=80 -- GitLab From 1e4c811214486972c52767d0c41e21b2fa60cd65 Mon Sep 17 00:00:00 2001 From: Douglas Raillard <douglas.raillard@arm.com> Date: Wed, 16 Aug 2023 17:48:36 +0100 Subject: [PATCH 2/2] tools/recipes/busybox.recipe: Fix insmod applet FIX Avoid using MODPROBE_SMALL=y that breaks the return value of insmod. Instead of returning the return code of the kernel module init function, it just returns 1. --- tools/recipes/busybox.recipe | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/recipes/busybox.recipe b/tools/recipes/busybox.recipe index d0c2fdc4d..988b5ea9d 100644 --- a/tools/recipes/busybox.recipe +++ b/tools/recipes/busybox.recipe @@ -11,8 +11,18 @@ download() { build() { cd busybox make defconfig - echo "CONFIG_STATIC=y" >> .config - make olddefconfig + + # We need to generate a defconfig then remove the config, then set them to + # the value we want, as there is no make olddefconfig to fixup an edited + # config. + cat .config | grep -v '\bCONFIG_MODPROBE_SMALL\b' | grep -v '\bCONFIG_STATIC\b' > myconfig + + echo "CONFIG_STATIC=y" >> myconfig + # MODPROBE_SMALL=y breaks the return code of insmod. Instead of forwarding + # the value from the kernel mod init function, it just returns 1. + echo "CONFIG_MODPROBE_SMALL=n" >> myconfig + + cp myconfig .config make -j 4 "CROSS_COMPILE=$CROSS_COMPILE" } -- GitLab