- Linux-kselftest-mirror - lists.linaro.org

[PATCH v5 1/3] Provide in-kernel headers to make extending kernel easier

by Joel Fernandes (Google)

Introduce in-kernel headers and other artifacts which are made available as an archive through proc (/proc/kheaders.tar.xz file). This archive makes it possible to build kernel modules, run eBPF programs, and other tracing programs that need to extend the kernel for tracing purposes without any dependency on the file system having headers and build artifacts. On Android and embedded systems, it is common to switch kernels but not have kernel headers available on the file system. Further once a different kernel is booted, any headers stored on the file system will no longer be useful. By storing the headers as a compressed archive within the kernel, we can avoid these issues that have been a hindrance for a long time. The best way to use this feature is by building it in. Several users have a need for this, when they switch debug kernels, they donot want to update the filesystem or worry about it where to store the headers on it. However, the feature is also buildable as a module in case the user desires it not being part of the kernel image. This makes it possible to load and unload the headers from memory on demand. A tracing program, or a kernel module builder can load the module, do its operations, and then unload the module to save kernel memory. The total memory needed is 3.8MB. By having the archive available at a fixed location independent of filesystem dependencies and conventions, all debugging tools can directly refer to the fixed location for the archive, without concerning with where the headers on a typical filesystem which significantly simplifies tooling that needs kernel headers. The code to read the headers is based on /proc/config.gz code and uses the same technique to embed the headers. To build a module, the below steps have been tested on an x86 machine: modprobe kheaders rm -rf $HOME/headers mkdir -p $HOME/headers tar -xvf /proc/kheaders.tar.xz -C $HOME/headers >/dev/null cd my-kernel-module make -C $HOME/headers M=$(pwd) modules rmmod kheaders Additional notes: (1) external modules must be built on the same arch as the host that built vmlinux. This can be done either in a qemu emulated chroot on the target, or natively. This is due to host arch dependency of kernel scripts. (2) If module building is used, since Module.symvers is not available in the archive due to a cyclic dependency with building of the archive into the kernel or module binaries, the modules built using the archive will not contain symbol versioning (modversion). This is usually not an issue since the idea of this patch is to build a kernel module on the fly and load it into the same kernel. An appropriate warning is already printed by the kernel to alert the user of modules not having modversions when built using the archive. For building with modversions, the user can use traditional header packages. For our tracing usecases, we build modules on the fly with this so it is not a concern. (3) I have left IKHD_ST and IKHD_ED markers as is to facilitate future patches that would extract the headers from a kernel or module image. (v4 was Tested-by the following folks, v5 only has minor changes and has passed my testing). Tested-by: qais.yousef(a)arm.com Tested-by: dietmar.eggemann(a)arm.com Tested-by: linux(a)manojrajarao.com Signed-off-by: Joel Fernandes (Google) <joel(a)joelfernandes.org> --- v4 -> v5: (Thanks to Masahiro Yamada for several excellent suggestions) - used incbin instead of bin2c (Masahiro did similar idea) - added module.lds if ia64 otherwise ia64 may fail to build. - added clean-files rule to Makefile - removed strip-comments script and doing it inline - added set -e to header generated to die on errorsr - fixed a minor issue where find command was noisy. - removed unneeded tar.xz rule from kernel/.gitignore - added Tested-by tags from ARM folks. Changes since v3: - Blank tar was being generated because of a one line I forgot to push. It is updated now. - Added module.lds since arm64 needs it to build modules. Changes since v2: (Thanks to Masahiro Yamada for several excellent suggestions) - Added support for out of tree builds. - Added incremental build support bringing down build time of incremental builds from 50 seconds to 5 seconds. - Fixed various small nits / cleanups. - clean ups to kheaders.c pointed by Alexey Dobriyan. - Fixed MODULE_LICENSE in test module and kheaders.c - Dropped Module.symvers from archive due to circular dependency. Changes since v1: - removed IKH_EXTRA variable, not needed (Masahiro Yamada) - small fix ups to selftest - added target to main Makefile etc - added MODULE_LICENSE to test module - made selftest more quiet Changes since RFC: Both changes bring size down to 3.8MB: - use xz for compression - strip comments except SPDX lines - Call out the module name in Kconfig - Also added selftests in second patch to ensure headers are always working. init/Kconfig | 11 ++++++ kernel/.gitignore | 1 + kernel/Makefile | 28 ++++++++++++++ kernel/kheaders.c | 73 +++++++++++++++++++++++++++++++++++ scripts/gen_ikh_data.sh | 84 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 197 insertions(+) create mode 100644 kernel/kheaders.c create mode 100755 scripts/gen_ikh_data.sh diff --git a/init/Kconfig b/init/Kconfig index 4592bf7997c0..ea75bfbf7dfa 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -580,6 +580,17 @@ config IKCONFIG_PROC This option enables access to the kernel configuration file through /proc/config.gz. +config IKHEADERS_PROC + tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz" + depends on PROC_FS + help + This option enables access to the kernel header and other artifacts that + are generated during the build process. These can be used to build kernel + modules or by other in-kernel programs such as those generated by eBPF + and systemtap tools. If you build the headers as a module, a module + called kheaders.ko is built which can be loaded on-demand to get access + to the headers. + config LOG_BUF_SHIFT int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" range 12 25 diff --git a/kernel/.gitignore b/kernel/.gitignore index 6e699100872f..34d1e77ee9df 100644 --- a/kernel/.gitignore +++ b/kernel/.gitignore @@ -1,5 +1,6 @@ # # Generated files # +kheaders.md5 timeconst.h hz.bc diff --git a/kernel/Makefile b/kernel/Makefile index 6c57e78817da..7c486bc25fd7 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_UTS_NS) += utsname.o obj-$(CONFIG_USER_NS) += user_namespace.o obj-$(CONFIG_PID_NS) += pid_namespace.o obj-$(CONFIG_IKCONFIG) += configs.o +obj-$(CONFIG_IKHEADERS_PROC) += kheaders.o obj-$(CONFIG_SMP) += stop_machine.o obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o obj-$(CONFIG_AUDIT) += audit.o auditfilter.o @@ -121,3 +122,30 @@ $(obj)/configs.o: $(obj)/config_data.gz targets += config_data.gz $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE $(call if_changed,gzip) + +# Build a list of in-kernel headers for building kernel modules +ikh_file_list := include/ +ikh_file_list += arch/$(SRCARCH)/Makefile +ikh_file_list += arch/$(SRCARCH)/include/ +ikh_file_list += arch/$(SRCARCH)/module.lds +ikh_file_list += arch/$(SRCARCH)/kernel/module.lds +ikh_file_list += scripts/ +ikh_file_list += Makefile + +# Things we need from the $objtree. "OBJDIR" is for the gen_ikh_data.sh +# script to identify that this comes from the $objtree directory +ikh_file_list += OBJDIR/scripts/ +ikh_file_list += OBJDIR/include/ +ikh_file_list += OBJDIR/arch/$(SRCARCH)/include/ +ifeq ($(CONFIG_STACK_VALIDATION), y) +ikh_file_list += OBJDIR/tools/objtool/objtool +endif + +$(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz + +quiet_cmd_genikh = GEN $(obj)/kheaders_data.tar.xz +cmd_genikh = $(srctree)/scripts/gen_ikh_data.sh $@ $(ikh_file_list) +$(obj)/kheaders_data.tar.xz: FORCE + $(call cmd,genikh) + +clean-files := kheaders_data.tar.xz kheaders.md5 diff --git a/kernel/kheaders.c b/kernel/kheaders.c new file mode 100644 index 000000000000..d072a958a8f1 --- /dev/null +++ b/kernel/kheaders.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * kernel/kheaders.c + * Provide headers and artifacts needed to build kernel modules. + * (Borrowed code from kernel/configs.c) + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/proc_fs.h> +#include <linux/init.h> +#include <linux/uaccess.h> + +/* + * Define kernel_headers_data and kernel_headers_data_end, within which the the + * compressed kernel headers are stpred. The file is first compressed with xz. + */ + +asm ( +" .pushsection .rodata, \"a\" \n" +" .global kernel_headers_data \n" +"kernel_headers_data: \n" +" .incbin \"kernel/kheaders_data.tar.xz\" \n" +" .global kernel_headers_data_end \n" +"kernel_headers_data_end: \n" +" .popsection \n" +); + +extern char kernel_headers_data; +extern char kernel_headers_data_end; + +static ssize_t +ikheaders_read_current(struct file *file, char __user *buf, + size_t len, loff_t *offset) +{ + return simple_read_from_buffer(buf, len, offset, + &kernel_headers_data, + &kernel_headers_data_end - + &kernel_headers_data); +} + +static const struct file_operations ikheaders_file_ops = { + .read = ikheaders_read_current, + .llseek = default_llseek, +}; + +static int __init ikheaders_init(void) +{ + struct proc_dir_entry *entry; + + /* create the current headers file */ + entry = proc_create("kheaders.tar.xz", S_IRUGO, NULL, + &ikheaders_file_ops); + if (!entry) + return -ENOMEM; + + proc_set_size(entry, + &kernel_headers_data_end - + &kernel_headers_data); + return 0; +} + +static void __exit ikheaders_cleanup(void) +{ + remove_proc_entry("kheaders.tar.xz", NULL); +} + +module_init(ikheaders_init); +module_exit(ikheaders_cleanup); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Joel Fernandes"); +MODULE_DESCRIPTION("Echo the kernel header artifacts used to build the kernel"); diff --git a/scripts/gen_ikh_data.sh b/scripts/gen_ikh_data.sh new file mode 100755 index 000000000000..3f9cae72c2a4 --- /dev/null +++ b/scripts/gen_ikh_data.sh @@ -0,0 +1,84 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# This script generates an archive consisting of kernel headers +# for CONFIG_IKHEADERS_PROC. +set -e + +spath="$(dirname "$(readlink -f "$0")")" +kroot="$spath/.." +outdir="$(pwd)" +tarfile=$1 +cpio_dir=$outdir/$tarfile.tmp + +file_list=${@:2} + +src_file_list="" +for f in $file_list; do + if [ ! -f "$kroot/$f" ] && [ ! -d "$kroot/$f" ]; then continue; fi + src_file_list="$src_file_list $(echo $f | grep -v OBJDIR)" +done + +obj_file_list="" +for f in $file_list; do + f=$(echo $f | grep OBJDIR | sed -e 's/OBJDIR\///g') + if [ ! -f $f ] && [ ! -d $f ]; then continue; fi + obj_file_list="$obj_file_list $f"; +done + +# Support incremental builds by skipping archive generation +# if timestamps of files being archived are not changed. + +# This block is useful for debugging the incremental builds. +# Uncomment it for debugging. +# iter=1 +# if [ ! -f /tmp/iter ]; then echo 1 > /tmp/iter; +# else; iter=$(($(cat /tmp/iter) + 1)); fi +# find $src_file_list -type f | xargs ls -lR > /tmp/src-ls-$iter +# find $obj_file_list -type f | xargs ls -lR > /tmp/obj-ls-$iter + +# modules.order and include/generated/compile.h are ignored because these are +# touched even when none of the source files changed. This causes pointless +# regeneration, so let us ignore them for md5 calculation. +pushd $kroot > /dev/null +src_files_md5="$(find $src_file_list -type f ! -name modules.order | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" +popd > /dev/null +obj_files_md5="$(find $obj_file_list -type f ! -name modules.order | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" + +if [ -f $tarfile ]; then tarfile_md5="$(md5sum $tarfile | cut -d ' ' -f1)"; fi +if [ -f kernel/kheaders.md5 ] && + [ "$(cat kernel/kheaders.md5|head -1)" == "$src_files_md5" ] && + [ "$(cat kernel/kheaders.md5|head -2|tail -1)" == "$obj_files_md5" ] && + [ "$(cat kernel/kheaders.md5|tail -1)" == "$tarfile_md5" ]; then + exit +fi + +rm -rf $cpio_dir +mkdir $cpio_dir + +pushd $kroot > /dev/null +for f in $src_file_list; + do find "$f" ! -name "*.c" ! -name "*.o" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir +popd > /dev/null + +# The second CPIO can complain if files already exist which can +# happen with out of tree builds. Just silence CPIO for now. +for f in $obj_file_list; + do find "$f" ! -name "*.c" ! -name "*.o" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1 + +find $cpio_dir -type f -print0 | + xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;' + +tar -Jcf $tarfile -C $cpio_dir/ . > /dev/null + +echo "$src_files_md5" > kernel/kheaders.md5 +echo "$obj_files_md5" >> kernel/kheaders.md5 +echo "$(md5sum $tarfile | cut -d ' ' -f1)" >> kernel/kheaders.md5 + +rm -rf $cpio_dir -- 2.21.0.225.g810b269d1ac-goog

6 years, 2 months

13
41
0 0

[PATCH] selftests: fix headers_install circular dependency

by Shuah Khan

"make kselftest" fails with "Circular Makefile.o <- prepare dependency dropped." error, when lib.mk invokes "make headers_install". Make level 0: Main make calls selftests run_tests target ... Make level n: selftests lib.mk invokes main make's headers_install The secondary level make inherits builtin-rules which will use the rule to generate Makefile.o and runs into "Circular Makefile.o <- prepare dependency dropped." error, and kselftest compile fails. Invoke headers_install target with --no-builtin-rules to avoid circular error. In addition, lib.mk installs headers in the default HDR_PATH, even when build relocation is requested with O= or export KBUILD_OUTPUT. Fix the problem by passing in INSTALL_HDR_PATH. The headers are installed under the specified output "dir/usr". Signed-off-by: Shuah Khan <shuah(a)kernel.org> --- tools/testing/selftests/Makefile | 52 +++++++++++++++++++++++++++----- tools/testing/selftests/lib.mk | 34 +++++++++++++++++++-- 2 files changed, 76 insertions(+), 10 deletions(-) diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile index 971fc8428117..743cd374d07d 100644 --- a/tools/testing/selftests/Makefile +++ b/tools/testing/selftests/Makefile @@ -75,12 +75,15 @@ ifneq ($(KBUILD_SRC),) override LDFLAGS = endif -BUILD := $(O) -ifndef BUILD - BUILD := $(KBUILD_OUTPUT) -endif -ifndef BUILD - BUILD := $(shell pwd) +ifneq ($(O),) + BUILD := $(O) +else + ifneq ($(KBUILD_OUTPUT),) + BUILD := $(KBUILD_OUTPUT) + else + BUILD := $(shell pwd) + DEFAULT_INSTALL_HDR_PATH := 1 + endif endif # KSFT_TAP_LEVEL is used from KSFT framework to prevent nested TAP header @@ -89,8 +92,41 @@ endif # with system() call. Export it here to cover override RUN_TESTS defines. export KSFT_TAP_LEVEL=`echo 1` +# Prepare for headers install +top_srcdir ?= ../../.. +include $(top_srcdir)/scripts/subarch.include +ARCH ?= $(SUBARCH) +export KSFT_KHDR_INSTALL_DONE := 1 export BUILD -all: + +# set default goal to all, so make without a target runs all, even when +# all isn't the first target in the file. +.DEFAULT_GOAL := all + +# Install headers here once for all tests. KSFT_KHDR_INSTALL_DONE +# is used to avoid running headers_install from lib.mk. +# Invoke headers install with --no-builtin-rules to avoid circular +# dependency in "make kselftest" case. In this case, second level +# make inherits builtin-rules which will use the rule generate +# Makefile.o and runs into +# "Circular Makefile.o <- prepare dependency dropped." +# and headers_install fails and test compile fails. +# +# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile +# invokes them as sub-makes and --no-builtin-rules is not necessary, +# but doesn't cause any failures. Keep it simple and use the same +# flags in both cases. +# Local build cases: "make kselftest", "make -C" - headers are installed +# in the default INSTALL_HDR_PATH usr/include. +khdr: +ifeq (1,$(DEFAULT_INSTALL_HDR_PATH)) + make --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install +else + make --no-builtin-rules INSTALL_HDR_PATH=$$BUILD/usr \ + ARCH=$(ARCH) -C $(top_srcdir) headers_install +endif + +all: khdr @for TARGET in $(TARGETS); do \ BUILD_TARGET=$$BUILD/$$TARGET; \ mkdir $$BUILD_TARGET -p; \ @@ -173,4 +209,4 @@ clean: make OUTPUT=$$BUILD_TARGET -C $$TARGET clean;\ done; -.PHONY: all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean +.PHONY: khdr all run_tests hotplug run_hotplug clean_hotplug run_pstore_crash install clean diff --git a/tools/testing/selftests/lib.mk b/tools/testing/selftests/lib.mk index 8b0f16409ed7..f888522a34e9 100644 --- a/tools/testing/selftests/lib.mk +++ b/tools/testing/selftests/lib.mk @@ -3,7 +3,16 @@ CC := $(CROSS_COMPILE)gcc ifeq (0,$(MAKELEVEL)) -OUTPUT := $(shell pwd) + ifneq ($(O),) + OUTPUT := $(O) + else + ifneq ($(KBUILD_OUTPUT),) + OUTPUT := $(KBUILD_OUTPUT) + else + OUTPUT := $(shell pwd) + DEFAULT_INSTALL_HDR_PATH := 1 + endif + endif endif # The following are built by lib.mk common compile rules. @@ -21,9 +30,30 @@ top_srcdir ?= ../../../.. include $(top_srcdir)/scripts/subarch.include ARCH ?= $(SUBARCH) +# Invoke headers install with --no-builtin-rules to avoid circular +# dependency in "make kselftest" case. In this case, second level +# make inherits builtin-rules which will use the rule generate +# Makefile.o and runs into +# "Circular Makefile.o <- prepare dependency dropped." +# and headers_install fails and test compile fails. +# O= KBUILD_OUTPUT cases don't run into this error, since main Makefile +# invokes them as sub-makes and --no-builtin-rules is not necessary, +# but doesn't cause any failures. Keep it simple and use the same +# flags in both cases. +# Note that the support to install headers from lib.mk is necessary +# when test Makefile is run directly with "make -C". +# When local build is done, headers are installed in the default +# INSTALL_HDR_PATH usr/include. .PHONY: khdr khdr: - make ARCH=$(ARCH) -C $(top_srcdir) headers_install +ifndef KSFT_KHDR_INSTALL_DONE +ifeq (1,$(DEFAULT_INSTALL_HDR_PATH)) + make --no-builtin-rules ARCH=$(ARCH) -C $(top_srcdir) headers_install +else + make --no-builtin-rules INSTALL_HDR_PATH=$$OUTPUT/usr \ + ARCH=$(ARCH) -C $(top_srcdir) headers_install +endif +endif all: khdr $(TEST_GEN_PROGS) $(TEST_GEN_PROGS_EXTENDED) $(TEST_GEN_FILES) else -- 2.17.1

6 years, 2 months

1
0
0 0

next-20190416 kselftest results

by ci_notify＠linaro.org

Summary ------------------------------------------------------------------------ git repo: not informed git branch: not informed git commit: not informed git describe: not informed Test details: https://qa-reports.linaro.org/lkft/linux-next-oe/build/next-20190416 Regressions (compared to build next-20190415) ------------------------------------------------------------------------ No regressions Fixes (compared to build next-20190415) ------------------------------------------------------------------------ No fixes In total: ------------------------------------------------------------------------ Ran 0 total tests in the following environments and test suites. pass 0 fail 0 xfail 0 skip 0 Environments -------------- - dragonboard-410c - arm64 - hi6220-hikey - arm64 - i386 - juno-r2 - arm64 - x15 - arm - x86_64 Test Suites ----------- Failures ------------------------------------------------------------------------ x86: i386: dragonboard-410c: hi6220-hikey: juno-r2: x15: Skips ------------------------------------------------------------------------ No skips -- Linaro LKFT https://lkft.linaro.org

6 years, 2 months

1
0
0 0

[PATCH] selftests: livepatch use TEST_PROGS for test shell scripts

by Shuah Khan

TEST_PROGS variable is for test shell scripts and common clean target in lib.mk doesn't touch them. TEST_GEN_PROGS are removed by it. Fix it to use TEST_PROGS for test shell scripts and TEST_PROGS_EXTENDED for common functions.sh. Signed-off-by: Shuah Khan <shuah(a)kernel.org> --- tools/testing/selftests/livepatch/Makefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/livepatch/Makefile b/tools/testing/selftests/livepatch/Makefile index af4aee79bebb..fd405402c3ff 100644 --- a/tools/testing/selftests/livepatch/Makefile +++ b/tools/testing/selftests/livepatch/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 -TEST_GEN_PROGS := \ +TEST_PROGS_EXTENDED := functions.sh +TEST_PROGS := \ test-livepatch.sh \ test-callbacks.sh \ test-shadow-vars.sh -- 2.17.1

6 years, 2 months

5
7
0 0

next-20190415 kselftest results

by ci_notify＠linaro.org

Summary ------------------------------------------------------------------------ git repo: not informed git branch: not informed git commit: not informed git describe: not informed Test details: https://qa-reports.linaro.org/lkft/linux-next-oe/build/next-20190415 Regressions (compared to build next-20190412) ------------------------------------------------------------------------ No regressions Fixes (compared to build next-20190412) ------------------------------------------------------------------------ No fixes In total: ------------------------------------------------------------------------ Ran 0 total tests in the following environments and test suites. pass 0 fail 0 xfail 0 skip 0 Environments -------------- - dragonboard-410c - arm64 - hi6220-hikey - arm64 - i386 - juno-r2 - arm64 - x15 - arm - x86_64 Test Suites ----------- Failures ------------------------------------------------------------------------ i386: juno-r2: hi6220-hikey: x15: x86: dragonboard-410c: Skips ------------------------------------------------------------------------ No skips -- Linaro LKFT https://lkft.linaro.org

6 years, 2 months

1
0
0 0

Fwd: [PATCH] selftests : netfilter: Wrote a error and exit code for a command which needed veth kernel module.

by Jeffrin Thalakkottoor

---------- Forwarded message --------- From: Jeffrin Thalakkottoor <jeffrin(a)rajagiritech.edu.in> Date: Sat, Apr 13, 2019 at 6:09 PM Subject: Re: [PATCH] selftests : netfilter: Wrote a error and exit code for a command which needed veth kernel module. To: Shuah Khan <shuah(a)kernel.org> Cc: Florian Westphal <fw(a)strlen.de>, <linu-kselftest(a)vger.kernel.org>, <pablo(a)netfilter.org>, lkml <linux-kernel(a)vger.kernel.org> Hello Shuah, did you get the mail related stuff below ? On Fri, Apr 5, 2019 at 10:17 PM Florian Westphal <fw(a)strlen.de> wrote: > > Jeffrin Jose T <jeffrin(a)rajagiritech.edu.in> wrote: > > A test for the basic NAT functionality uses ip command which > > needs veth device.There is a condition where the kernel support > > for veth is not compiled into the kernel and the test script > > breaks.This patch contains code for reasonable error display > > and correct code exit. > > Looks good to me, thanks for following up on this. > > Acked-by: Florian Westphal <fw(a)strlen.de> -- software engineer rajagiri school of engineering and technology -- software engineer rajagiri school of engineering and technology

6 years, 2 months

1
0
0 0

[PATCH v6 1/2] Provide in-kernel headers to make extending kernel easier

by Joel Fernandes (Google)

Introduce in-kernel headers which are made available as an archive through proc (/proc/kheaders.tar.xz file). This archive makes it possible to run eBPF and other tracing programs tracing programs that need to extend the kernel for tracing purposes without any dependency on the file system having headers. On Android and embedded systems, it is common to switch kernels but not have kernel headers available on the file system. Further once a different kernel is booted, any headers stored on the file system will no longer be useful. By storing the headers as a compressed archive within the kernel, we can avoid these issues that have been a hindrance for a long time. The best way to use this feature is by building it in. Several users have a need for this, when they switch debug kernels, they donot want to update the filesystem or worry about it where to store the headers on it. However, the feature is also buildable as a module in case the user desires it not being part of the kernel image. This makes it possible to load and unload the headers from memory on demand. A tracing program, or a kernel module builder can load the module, do its operations, and then unload the module to save kernel memory. The total memory needed is 3.8MB. By having the archive available at a fixed location independent of filesystem dependencies and conventions, all debugging tools can directly refer to the fixed location for the archive, without concerning with where the headers on a typical filesystem which significantly simplifies tooling that needs kernel headers. The code to read the headers is based on /proc/config.gz code and uses the same technique to embed the headers. IKHD_ST and IKHD_ED markers as is to facilitate future patches that would extract the headers from a kernel or module image. Signed-off-by: Joel Fernandes (Google) <joel(a)joelfernandes.org> --- v5 -> v6: (Masahiro Yamada suggestions mostly) - Dropped support for module building. - Rebuild archive if script changes. - Move archive file list to script. - Move build script to kernel directory. v4 -> v5: (v4 was Tested-by the following folks) Tested-by: qais.yousef(a)arm.com Tested-by: dietmar.eggemann(a)arm.com Tested-by: linux(a)manojrajarao.com (Thanks to Masahiro Yamada for several excellent suggestions) - used incbin instead of bin2c (Masahiro did similar idea) - added module.lds if ia64 otherwise ia64 may fail to build. - added clean-files rule to Makefile - removed strip-comments script and doing it inline - added set -e to header generated to die on errorsr - fixed a minor issue where find command was noisy. - removed unneeded tar.xz rule from kernel/.gitignore - added Tested-by tags from ARM folks. Changes since v3: - Blank tar was being generated because of a one line I forgot to push. It is updated now. - Added module.lds since arm64 needs it to build modules. Changes since v2: (Thanks to Masahiro Yamada for several excellent suggestions) - Added support for out of tree builds. - Added incremental build support bringing down build time of incremental builds from 50 seconds to 5 seconds. - Fixed various small nits / cleanups. - clean ups to kheaders.c pointed by Alexey Dobriyan. - Fixed MODULE_LICENSE in test module and kheaders.c - Dropped Module.symvers from archive due to circular dependency. Changes since v1: - removed IKH_EXTRA variable, not needed (Masahiro Yamada) - small fix ups to selftest - added target to main Makefile etc - added MODULE_LICENSE to test module - made selftest more quiet Changes since RFC: Both changes bring size down to 3.8MB: - use xz for compression - strip comments except SPDX lines - Call out the module name in Kconfig - Also added selftests in second patch to ensure headers are always working. Signed-off-by: Joel Fernandes (Google) <joel(a)joelfernandes.org> init/Kconfig | 11 ++++++ kernel/.gitignore | 1 + kernel/Makefile | 10 +++++ kernel/gen_ikh_data.sh | 84 ++++++++++++++++++++++++++++++++++++++++++ kernel/kheaders.c | 73 ++++++++++++++++++++++++++++++++++++ 5 files changed, 179 insertions(+) create mode 100755 kernel/gen_ikh_data.sh create mode 100644 kernel/kheaders.c diff --git a/init/Kconfig b/init/Kconfig index 4592bf7997c0..ea75bfbf7dfa 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -580,6 +580,17 @@ config IKCONFIG_PROC This option enables access to the kernel configuration file through /proc/config.gz. +config IKHEADERS_PROC + tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz" + depends on PROC_FS + help + This option enables access to the kernel header and other artifacts that + are generated during the build process. These can be used to build kernel + modules or by other in-kernel programs such as those generated by eBPF + and systemtap tools. If you build the headers as a module, a module + called kheaders.ko is built which can be loaded on-demand to get access + to the headers. + config LOG_BUF_SHIFT int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" range 12 25 diff --git a/kernel/.gitignore b/kernel/.gitignore index 6e699100872f..34d1e77ee9df 100644 --- a/kernel/.gitignore +++ b/kernel/.gitignore @@ -1,5 +1,6 @@ # # Generated files # +kheaders.md5 timeconst.h hz.bc diff --git a/kernel/Makefile b/kernel/Makefile index 6c57e78817da..e3c581d8cde7 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_UTS_NS) += utsname.o obj-$(CONFIG_USER_NS) += user_namespace.o obj-$(CONFIG_PID_NS) += pid_namespace.o obj-$(CONFIG_IKCONFIG) += configs.o +obj-$(CONFIG_IKHEADERS_PROC) += kheaders.o obj-$(CONFIG_SMP) += stop_machine.o obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o obj-$(CONFIG_AUDIT) += audit.o auditfilter.o @@ -121,3 +122,12 @@ $(obj)/configs.o: $(obj)/config_data.gz targets += config_data.gz $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE $(call if_changed,gzip) + +$(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz + +quiet_cmd_genikh = GEN $(obj)/kheaders_data.tar.xz +cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@ +$(obj)/kheaders_data.tar.xz: FORCE + $(call cmd,genikh) + +clean-files := kheaders_data.tar.xz kheaders.md5 diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_ikh_data.sh new file mode 100755 index 000000000000..ef72c2740d01 --- /dev/null +++ b/kernel/gen_ikh_data.sh @@ -0,0 +1,84 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# This script generates an archive consisting of kernel headers +# for CONFIG_IKHEADERS_PROC. +set -e +spath="$(dirname "$(readlink -f "$0")")" +kroot="$spath/.." +outdir="$(pwd)" +tarfile=$1 +cpio_dir=$outdir/$tarfile.tmp + +# Script filename relative to the kernel source root +# We add it to the archive because it is small and any changes +# to this script will also cause a rebuild of the archive. +sfile="$(realpath --relative-to $kroot "$(readlink -f "$0")")" + +src_file_list=" +include/ +arch/$SRCARCH/include/ +$sfile +" + +obj_file_list=" +include/ +arch/$SRCARCH/include/ +" + +# Support incremental builds by skipping archive generation +# if timestamps of files being archived are not changed. + +# This block is useful for debugging the incremental builds. +# Uncomment it for debugging. +# iter=1 +# if [ ! -f /tmp/iter ]; then echo 1 > /tmp/iter; +# else; iter=$(($(cat /tmp/iter) + 1)); fi +# find $src_file_list -type f | xargs ls -lR > /tmp/src-ls-$iter +# find $obj_file_list -type f | xargs ls -lR > /tmp/obj-ls-$iter + +# include/generated/compile.h is ignored because it is touched even when none +# of the source files changed. This causes pointless regeneration, so let us +# ignore them for md5 calculation. +pushd $kroot > /dev/null +src_files_md5="$(find $src_file_list -type f | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" +popd > /dev/null +obj_files_md5="$(find $obj_file_list -type f | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" + +if [ -f $tarfile ]; then tarfile_md5="$(md5sum $tarfile | cut -d ' ' -f1)"; fi +if [ -f kernel/kheaders.md5 ] && + [ "$(cat kernel/kheaders.md5|head -1)" == "$src_files_md5" ] && + [ "$(cat kernel/kheaders.md5|head -2|tail -1)" == "$obj_files_md5" ] && + [ "$(cat kernel/kheaders.md5|tail -1)" == "$tarfile_md5" ]; then + exit +fi + +rm -rf $cpio_dir +mkdir $cpio_dir + +pushd $kroot > /dev/null +for f in $src_file_list; + do find "$f" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir +popd > /dev/null + +# The second CPIO can complain if files already exist which can +# happen with out of tree builds. Just silence CPIO for now. +for f in $obj_file_list; + do find "$f" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1 + +find $cpio_dir -type f -print0 | + xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;' + +tar -Jcf $tarfile -C $cpio_dir/ . > /dev/null + +echo "$src_files_md5" > kernel/kheaders.md5 +echo "$obj_files_md5" >> kernel/kheaders.md5 +echo "$(md5sum $tarfile | cut -d ' ' -f1)" >> kernel/kheaders.md5 + +rm -rf $cpio_dir diff --git a/kernel/kheaders.c b/kernel/kheaders.c new file mode 100644 index 000000000000..d072a958a8f1 --- /dev/null +++ b/kernel/kheaders.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * kernel/kheaders.c + * Provide headers and artifacts needed to build kernel modules. + * (Borrowed code from kernel/configs.c) + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/proc_fs.h> +#include <linux/init.h> +#include <linux/uaccess.h> + +/* + * Define kernel_headers_data and kernel_headers_data_end, within which the the + * compressed kernel headers are stpred. The file is first compressed with xz. + */ + +asm ( +" .pushsection .rodata, \"a\" \n" +" .global kernel_headers_data \n" +"kernel_headers_data: \n" +" .incbin \"kernel/kheaders_data.tar.xz\" \n" +" .global kernel_headers_data_end \n" +"kernel_headers_data_end: \n" +" .popsection \n" +); + +extern char kernel_headers_data; +extern char kernel_headers_data_end; + +static ssize_t +ikheaders_read_current(struct file *file, char __user *buf, + size_t len, loff_t *offset) +{ + return simple_read_from_buffer(buf, len, offset, + &kernel_headers_data, + &kernel_headers_data_end - + &kernel_headers_data); +} + +static const struct file_operations ikheaders_file_ops = { + .read = ikheaders_read_current, + .llseek = default_llseek, +}; + +static int __init ikheaders_init(void) +{ + struct proc_dir_entry *entry; + + /* create the current headers file */ + entry = proc_create("kheaders.tar.xz", S_IRUGO, NULL, + &ikheaders_file_ops); + if (!entry) + return -ENOMEM; + + proc_set_size(entry, + &kernel_headers_data_end - + &kernel_headers_data); + return 0; +} + +static void __exit ikheaders_cleanup(void) +{ + remove_proc_entry("kheaders.tar.xz", NULL); +} + +module_init(ikheaders_init); +module_exit(ikheaders_cleanup); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Joel Fernandes"); +MODULE_DESCRIPTION("Echo the kernel header artifacts used to build the kernel"); -- 2.21.0.392.gf8f6787159e-goog

6 years, 2 months

5
11
0 0

[PATCH v7 1/2] Provide in-kernel headers to make extending kernel easier

by Joel Fernandes (Google)

Introduce in-kernel headers which are made available as an archive through proc (/proc/kheaders.tar.xz file). This archive makes it possible to run eBPF and other tracing programs that need to extend the kernel for tracing purposes without any dependency on the file system having headers. A github PR is sent for the corresponding BCC patch at: https://github.com/iovisor/bcc/pull/2312 On Android and embedded systems, it is common to switch kernels but not have kernel headers available on the file system. Further once a different kernel is booted, any headers stored on the file system will no longer be useful. This is an issue even well known to distros. By storing the headers as a compressed archive within the kernel, we can avoid these issues that have been a hindrance for a long time. The best way to use this feature is by building it in. Several users have a need for this, when they switch debug kernels, they do not want to update the filesystem or worry about it where to store the headers on it. However, the feature is also buildable as a module in case the user desires it not being part of the kernel image. This makes it possible to load and unload the headers from memory on demand. A tracing program can load the module, do its operations, and then unload the module to save kernel memory. The total memory needed is 3.3MB. By having the archive available at a fixed location independent of filesystem dependencies and conventions, all debugging tools can directly refer to the fixed location for the archive, without concerning with where the headers on a typical filesystem which significantly simplifies tooling that needs kernel headers. The code to read the headers is based on /proc/config.gz code and uses the same technique to embed the headers. Other approaches were discussed such as having an in-memory mountable filesystem, but that has drawbacks such as requiring an in-kernel xz decompressor which we don't have today, and requiring usage of 42 MB of kernel memory to host the decompressed headers at anytime. Also this approach is simpler than such approaches. Signed-off-by: Joel Fernandes (Google) <joel(a)joelfernandes.org> --- v6 -> v7: - Minor nits from Masahiro Yamada are addressed. v5 -> v6: (Masahiro Yamada suggestions mostly) - Dropped support for module building. - Rebuild archive if script changes. - Move archive file list to script. - Move build script to kernel directory. v4 -> v5: (v4 was Tested-by the following folks) Tested-by: qais.yousef(a)arm.com Tested-by: dietmar.eggemann(a)arm.com Tested-by: linux(a)manojrajarao.com (Thanks to Masahiro Yamada for several excellent suggestions) - used incbin instead of bin2c (Masahiro did similar idea) - added module.lds if ia64 otherwise ia64 may fail to build. - added clean-files rule to Makefile - removed strip-comments script and doing it inline - added set -e to header generated to die on errorsr - fixed a minor issue where find command was noisy. - removed unneeded tar.xz rule from kernel/.gitignore - added Tested-by tags from ARM folks. Changes since v3: - Blank tar was being generated because of a one line I forgot to push. It is updated now. - Added module.lds since arm64 needs it to build modules. Changes since v2: (Thanks to Masahiro Yamada for several excellent suggestions) - Added support for out of tree builds. - Added incremental build support bringing down build time of incremental builds from 50 seconds to 5 seconds. - Fixed various small nits / cleanups. - clean ups to kheaders.c pointed by Alexey Dobriyan. - Fixed MODULE_LICENSE in test module and kheaders.c - Dropped Module.symvers from archive due to circular dependency. Changes since v1: - removed IKH_EXTRA variable, not needed (Masahiro Yamada) - small fix ups to selftest - added target to main Makefile etc - added MODULE_LICENSE to test module - made selftest more quiet Changes since RFC: Both changes bring size down to 3.8MB: - use xz for compression - strip comments except SPDX lines - Call out the module name in Kconfig - Also added selftests in second patch to ensure headers are always working. Signed-off-by: Joel Fernandes (Google) <joel(a)joelfernandes.org> init/Kconfig | 10 +++++ kernel/.gitignore | 1 + kernel/Makefile | 10 +++++ kernel/gen_ikh_data.sh | 89 ++++++++++++++++++++++++++++++++++++++++++ kernel/kheaders.c | 74 +++++++++++++++++++++++++++++++++++ 5 files changed, 184 insertions(+) create mode 100755 kernel/gen_ikh_data.sh create mode 100644 kernel/kheaders.c diff --git a/init/Kconfig b/init/Kconfig index 4592bf7997c0..47c0db6e63a5 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -580,6 +580,16 @@ config IKCONFIG_PROC This option enables access to the kernel configuration file through /proc/config.gz. +config IKHEADERS_PROC + tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz" + depends on PROC_FS + help + This option enables access to the kernel header and other artifacts that + are generated during the build process. These can be used to build eBPF + tracing programs, or similar programs. If you build the headers as a + module, a module called kheaders.ko is built which can be loaded on-demand + to get access to the headers. + config LOG_BUF_SHIFT int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" range 12 25 diff --git a/kernel/.gitignore b/kernel/.gitignore index 6e699100872f..34d1e77ee9df 100644 --- a/kernel/.gitignore +++ b/kernel/.gitignore @@ -1,5 +1,6 @@ # # Generated files # +kheaders.md5 timeconst.h hz.bc diff --git a/kernel/Makefile b/kernel/Makefile index 6c57e78817da..12399614c350 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_UTS_NS) += utsname.o obj-$(CONFIG_USER_NS) += user_namespace.o obj-$(CONFIG_PID_NS) += pid_namespace.o obj-$(CONFIG_IKCONFIG) += configs.o +obj-$(CONFIG_IKHEADERS_PROC) += kheaders.o obj-$(CONFIG_SMP) += stop_machine.o obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o obj-$(CONFIG_AUDIT) += audit.o auditfilter.o @@ -121,3 +122,12 @@ $(obj)/configs.o: $(obj)/config_data.gz targets += config_data.gz $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE $(call if_changed,gzip) + +$(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz + +quiet_cmd_genikh = CHK $(obj)/kheaders_data.tar.xz +cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@ +$(obj)/kheaders_data.tar.xz: FORCE + $(call cmd,genikh) + +clean-files := kheaders_data.tar.xz kheaders.md5 diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_ikh_data.sh new file mode 100755 index 000000000000..591a94f7b387 --- /dev/null +++ b/kernel/gen_ikh_data.sh @@ -0,0 +1,89 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# This script generates an archive consisting of kernel headers +# for CONFIG_IKHEADERS_PROC. +set -e +spath="$(dirname "$(readlink -f "$0")")" +kroot="$spath/.." +outdir="$(pwd)" +tarfile=$1 +cpio_dir=$outdir/$tarfile.tmp + +# Script filename relative to the kernel source root +# We add it to the archive because it is small and any changes +# to this script will also cause a rebuild of the archive. +sfile="$(realpath --relative-to $kroot "$(readlink -f "$0")")" + +src_file_list=" +include/ +arch/$SRCARCH/include/ +$sfile +" + +obj_file_list=" +include/ +arch/$SRCARCH/include/ +" + +# Support incremental builds by skipping archive generation +# if timestamps of files being archived are not changed. + +# This block is useful for debugging the incremental builds. +# Uncomment it for debugging. +# iter=1 +# if [ ! -f /tmp/iter ]; then echo 1 > /tmp/iter; +# else; iter=$(($(cat /tmp/iter) + 1)); fi +# find $src_file_list -type f | xargs ls -lR > /tmp/src-ls-$iter +# find $obj_file_list -type f | xargs ls -lR > /tmp/obj-ls-$iter + +# include/generated/compile.h is ignored because it is touched even when none +# of the source files changed. This causes pointless regeneration, so let us +# ignore them for md5 calculation. +pushd $kroot > /dev/null +src_files_md5="$(find $src_file_list -type f | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" +popd > /dev/null +obj_files_md5="$(find $obj_file_list -type f | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" + +if [ -f $tarfile ]; then tarfile_md5="$(md5sum $tarfile | cut -d ' ' -f1)"; fi +if [ -f kernel/kheaders.md5 ] && + [ "$(cat kernel/kheaders.md5|head -1)" == "$src_files_md5" ] && + [ "$(cat kernel/kheaders.md5|head -2|tail -1)" == "$obj_files_md5" ] && + [ "$(cat kernel/kheaders.md5|tail -1)" == "$tarfile_md5" ]; then + exit +fi + +if [ "${quiet}" != "silent_" ]; then + echo " GEN $tarfile" +fi + +rm -rf $cpio_dir +mkdir $cpio_dir + +pushd $kroot > /dev/null +for f in $src_file_list; + do find "$f" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir +popd > /dev/null + +# The second CPIO can complain if files already exist which can +# happen with out of tree builds. Just silence CPIO for now. +for f in $obj_file_list; + do find "$f" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1 + +# Remove comments except SDPX lines +find $cpio_dir -type f -print0 | + xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;' + +tar -Jcf $tarfile -C $cpio_dir/ . > /dev/null + +echo "$src_files_md5" > kernel/kheaders.md5 +echo "$obj_files_md5" >> kernel/kheaders.md5 +echo "$(md5sum $tarfile | cut -d ' ' -f1)" >> kernel/kheaders.md5 + +rm -rf $cpio_dir diff --git a/kernel/kheaders.c b/kernel/kheaders.c new file mode 100644 index 000000000000..19526294a4e8 --- /dev/null +++ b/kernel/kheaders.c @@ -0,0 +1,74 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Provide kernel headers useful to build tracing programs + * such as for running eBPF tracing tools. + * + * (Borrowed code from kernel/configs.c) + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/proc_fs.h> +#include <linux/init.h> +#include <linux/uaccess.h> + +/* + * Define kernel_headers_data and kernel_headers_data_end, within which the the + * compressed kernel headers are stpred. The file is first compressed with xz. + */ + +asm ( +" .pushsection .rodata, \"a\" \n" +" .global kernel_headers_data \n" +"kernel_headers_data: \n" +" .incbin \"kernel/kheaders_data.tar.xz\" \n" +" .global kernel_headers_data_end \n" +"kernel_headers_data_end: \n" +" .popsection \n" +); + +extern char kernel_headers_data; +extern char kernel_headers_data_end; + +static ssize_t +ikheaders_read_current(struct file *file, char __user *buf, + size_t len, loff_t *offset) +{ + return simple_read_from_buffer(buf, len, offset, + &kernel_headers_data, + &kernel_headers_data_end - + &kernel_headers_data); +} + +static const struct file_operations ikheaders_file_ops = { + .read = ikheaders_read_current, + .llseek = default_llseek, +}; + +static int __init ikheaders_init(void) +{ + struct proc_dir_entry *entry; + + /* create the current headers file */ + entry = proc_create("kheaders.tar.xz", S_IRUGO, NULL, + &ikheaders_file_ops); + if (!entry) + return -ENOMEM; + + proc_set_size(entry, + &kernel_headers_data_end - + &kernel_headers_data); + return 0; +} + +static void __exit ikheaders_cleanup(void) +{ + remove_proc_entry("kheaders.tar.xz", NULL); +} + +module_init(ikheaders_init); +module_exit(ikheaders_cleanup); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Joel Fernandes"); +MODULE_DESCRIPTION("Echo the kernel header artifacts used to build the kernel"); -- 2.21.0.392.gf8f6787159e-goog

6 years, 2 months

2
2
0 0

Re: [PATCH RFC 1/2] Add polling support to pidfd

by Daniel Colascione

[Resending due to accidental HTML. I need to take Joel's advice and switch to a real email client] On Fri, Apr 12, 2019 at 5:54 PM Daniel Colascione <dancol(a)google.com> wrote: > > On Fri, Apr 12, 2019 at 5:09 PM Joel Fernandes <joel(a)joelfernandes.org> wrote: >> >> Hi Andy! >> >> On Fri, Apr 12, 2019 at 02:32:53PM -0700, Andy Lutomirski wrote: >> > On Thu, Apr 11, 2019 at 10:51 AM Joel Fernandes (Google) >> > <joel(a)joelfernandes.org> wrote: >> > > >> > > pidfd are /proc/pid directory file descriptors referring to a task group >> > > leader. Android low memory killer (LMK) needs pidfd polling support to >> > > replace code that currently checks for existence of /proc/pid for >> > > knowing a process that is signalled to be killed has died, which is both >> > > racy and slow. The pidfd poll approach is race-free, and also allows the >> > > LMK to do other things (such as by polling on other fds) while awaiting >> > > the process being killed to die. >> > > >> > > It prevents a situation where a PID is reused between when LMK sends a >> > > kill signal and checks for existence of the PID, since the wrong PID is >> > > now possibly checked for existence. >> > > >> > > In this patch, we follow the same mechanism used uhen the parent of the >> > > task group is to be notified, that is when the tasks waiting on a poll >> > > of pidfd are also awakened. >> > > >> > > We have decided to include the waitqueue in struct pid for the following >> > > reasons: >> > > 1. The wait queue has to survive for the lifetime of the poll. Including >> > > it in task_struct would not be option in this case because the task can >> > > be reaped and destroyed before the poll returns. >> > >> > Are you sure? I admit I'm not all that familiar with the innards of >> > poll() on Linux, but I thought that the waitqueue only had to survive >> > long enough to kick the polling thread and did *not* have to survive >> > until poll() actually returned. >> >> I am not sure now. I thought epoll(2) was based on the wait_event APIs, >> however more closely looking at the eventpoll code, it looks like there are 2 >> waitqueues involved, one that we pass and the other that is a part of the >> eventpoll session itself, so you could be right about that. Daniel Colascione >> may have some more thoughts about it since he brought up the possiblity of a >> wq life-time issue. Daniel? We were just playing it safe. I think you (Joel) and Andy are talking about different meanings of poll(). Joel is talking about the VFS method; Andy is talking about the system call. ISTM that the lifetime of wait queue we give to poll_wait needs to last through the poll. Normally the wait queue gets pinned by the struct file that we give to poll_wait (which takes a reference on the struct file), but the pidfd struct file doesn't pin the struct task, so we can't use a wait queue in struct task. (remove_wait_queue, which poll implementations call to undo wait queue additions, takes the wait queue head we pass to poll_wait, and we don't want to pass a dangling pointer to remove_wait_queue.) If the lifetime requirements for the queue aren't this strict, I don't see it documented anywhere. Besides: if we don't actually need to pin the waitqueue lifetime for the duration of the poll, why bother taking a reference on the polled struct file?

6 years, 2 months

1
0
0 0

[PATCH] selftests/seccomp: Handle namespace failures gracefully

by Kees Cook

When running without USERNS or PIDNS the seccomp test would hang since it was waiting forever for the child to trigger the user notification since it seems the glibc() abort handler makes a call to getpid(), which would trap again. This changes the getpid filter to getppid, and makes sure ASSERTs execute to stop from spawning the listener. Reported-by: Shuah Khan <shuah(a)kernel.org> Fixes: 6a21cc50f0c7 ("seccomp: add a return code to trap to userspace") Signed-off-by: Kees Cook <keescook(a)chromium.org> --- tools/testing/selftests/seccomp/seccomp_bpf.c | 43 ++++++++++--------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c index f69d2ee29742..3a280b7efc87 100644 --- a/tools/testing/selftests/seccomp/seccomp_bpf.c +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c @@ -3079,9 +3079,9 @@ TEST(user_notification_basic) /* Check that we get -ENOSYS with no listener attached */ if (pid == 0) { - if (user_trap_syscall(__NR_getpid, 0) < 0) + if (user_trap_syscall(__NR_getppid, 0) < 0) exit(1); - ret = syscall(__NR_getpid); + ret = syscall(__NR_getppid); exit(ret >= 0 || errno != ENOSYS); } @@ -3096,12 +3096,12 @@ TEST(user_notification_basic) EXPECT_EQ(seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog), 0); /* Check that the basic notification machinery works */ - listener = user_trap_syscall(__NR_getpid, + listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); /* Installing a second listener in the chain should EBUSY */ - EXPECT_EQ(user_trap_syscall(__NR_getpid, + EXPECT_EQ(user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER), -1); EXPECT_EQ(errno, EBUSY); @@ -3110,7 +3110,7 @@ TEST(user_notification_basic) ASSERT_GE(pid, 0); if (pid == 0) { - ret = syscall(__NR_getpid); + ret = syscall(__NR_getppid); exit(ret != USER_NOTIF_MAGIC); } @@ -3128,7 +3128,7 @@ TEST(user_notification_basic) EXPECT_GT(poll(&pollfd, 1, -1), 0); EXPECT_EQ(pollfd.revents, POLLOUT); - EXPECT_EQ(req.data.nr, __NR_getpid); + EXPECT_EQ(req.data.nr, __NR_getppid); resp.id = req.id; resp.error = 0; @@ -3160,7 +3160,7 @@ TEST(user_notification_kill_in_middle) TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); } - listener = user_trap_syscall(__NR_getpid, + listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); @@ -3172,7 +3172,7 @@ TEST(user_notification_kill_in_middle) ASSERT_GE(pid, 0); if (pid == 0) { - ret = syscall(__NR_getpid); + ret = syscall(__NR_getppid); exit(ret != USER_NOTIF_MAGIC); } @@ -3282,7 +3282,7 @@ TEST(user_notification_closed_listener) TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); } - listener = user_trap_syscall(__NR_getpid, + listener = user_trap_syscall(__NR_getppid, SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); @@ -3293,7 +3293,7 @@ TEST(user_notification_closed_listener) ASSERT_GE(pid, 0); if (pid == 0) { close(listener); - ret = syscall(__NR_getpid); + ret = syscall(__NR_getppid); exit(ret != -1 && errno != ENOSYS); } @@ -3316,14 +3316,15 @@ TEST(user_notification_child_pid_ns) ASSERT_EQ(unshare(CLONE_NEWUSER | CLONE_NEWPID), 0); - listener = user_trap_syscall(__NR_getpid, SECCOMP_FILTER_FLAG_NEW_LISTENER); + listener = user_trap_syscall(__NR_getppid, + SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); pid = fork(); ASSERT_GE(pid, 0); if (pid == 0) - exit(syscall(__NR_getpid) != USER_NOTIF_MAGIC); + exit(syscall(__NR_getppid) != USER_NOTIF_MAGIC); EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); EXPECT_EQ(req.pid, pid); @@ -3355,7 +3356,8 @@ TEST(user_notification_sibling_pid_ns) TH_LOG("Kernel does not support PR_SET_NO_NEW_PRIVS!"); } - listener = user_trap_syscall(__NR_getpid, SECCOMP_FILTER_FLAG_NEW_LISTENER); + listener = user_trap_syscall(__NR_getppid, + SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); pid = fork(); @@ -3368,7 +3370,7 @@ TEST(user_notification_sibling_pid_ns) ASSERT_GE(pid2, 0); if (pid2 == 0) - exit(syscall(__NR_getpid) != USER_NOTIF_MAGIC); + exit(syscall(__NR_getppid) != USER_NOTIF_MAGIC); EXPECT_EQ(waitpid(pid2, &status, 0), pid2); EXPECT_EQ(true, WIFEXITED(status)); @@ -3377,11 +3379,11 @@ TEST(user_notification_sibling_pid_ns) } /* Create the sibling ns, and sibling in it. */ - EXPECT_EQ(unshare(CLONE_NEWPID), 0); - EXPECT_EQ(errno, 0); + ASSERT_EQ(unshare(CLONE_NEWPID), 0); + ASSERT_EQ(errno, 0); pid2 = fork(); - EXPECT_GE(pid2, 0); + ASSERT_GE(pid2, 0); if (pid2 == 0) { ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0); @@ -3389,7 +3391,7 @@ TEST(user_notification_sibling_pid_ns) * The pid should be 0, i.e. the task is in some namespace that * we can't "see". */ - ASSERT_EQ(req.pid, 0); + EXPECT_EQ(req.pid, 0); resp.id = req.id; resp.error = 0; @@ -3419,14 +3421,15 @@ TEST(user_notification_fault_recv) ASSERT_EQ(unshare(CLONE_NEWUSER), 0); - listener = user_trap_syscall(__NR_getpid, SECCOMP_FILTER_FLAG_NEW_LISTENER); + listener = user_trap_syscall(__NR_getppid, + SECCOMP_FILTER_FLAG_NEW_LISTENER); ASSERT_GE(listener, 0); pid = fork(); ASSERT_GE(pid, 0); if (pid == 0) - exit(syscall(__NR_getpid) != USER_NOTIF_MAGIC); + exit(syscall(__NR_getppid) != USER_NOTIF_MAGIC); /* Do a bad recv() */ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, NULL), -1); -- 2.17.1 -- Kees Cook

6 years, 2 months

3
3
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror