LinuxLists.cc - [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

[permalink] [raw]

Subject: [PATCH v4 2/2] Add selftests for module build using in-kernel headers

This test tries to build a module successfully using the in-kernel
headers found in /proc/kheaders.tar.xz.

Verified pass and fail scenarios by running:
make -C tools/testing/selftests TARGETS=kheaders run_tests

Signed-off-by: Joel Fernandes (Google) <[email protected]>
---
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/kheaders/Makefile | 5 +++++
tools/testing/selftests/kheaders/config | 1 +
.../kheaders/run_kheaders_modbuild.sh | 18 +++++++++++++++++
.../selftests/kheaders/testmod/Makefile | 3 +++
.../testing/selftests/kheaders/testmod/test.c | 20 +++++++++++++++++++
6 files changed, 48 insertions(+)
create mode 100644 tools/testing/selftests/kheaders/Makefile
create mode 100644 tools/testing/selftests/kheaders/config
create mode 100755 tools/testing/selftests/kheaders/run_kheaders_modbuild.sh
create mode 100644 tools/testing/selftests/kheaders/testmod/Makefile
create mode 100644 tools/testing/selftests/kheaders/testmod/test.c

diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 400ee81a3043..5a9287fddd0d 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -20,6 +20,7 @@ TARGETS += intel_pstate
TARGETS += ipc
TARGETS += ir
TARGETS += kcmp
+TARGETS += kheaders
TARGETS += kvm
TARGETS += lib
TARGETS += membarrier
diff --git a/tools/testing/selftests/kheaders/Makefile b/tools/testing/selftests/kheaders/Makefile
new file mode 100644
index 000000000000..51035ab0732b
--- /dev/null
+++ b/tools/testing/selftests/kheaders/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+TEST_PROGS := run_kheaders_modbuild.sh
+
+include ../lib.mk
diff --git a/tools/testing/selftests/kheaders/config b/tools/testing/selftests/kheaders/config
new file mode 100644
index 000000000000..5221f9fb5e79
--- /dev/null
+++ b/tools/testing/selftests/kheaders/config
@@ -0,0 +1 @@
+CONFIG_IKHEADERS_PROC=y
diff --git a/tools/testing/selftests/kheaders/run_kheaders_modbuild.sh b/tools/testing/selftests/kheaders/run_kheaders_modbuild.sh
new file mode 100755
index 000000000000..f001568e08b0
--- /dev/null
+++ b/tools/testing/selftests/kheaders/run_kheaders_modbuild.sh
@@ -0,0 +1,18 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+
+HEADERS_XZ=/proc/kheaders.tar.xz
+TMP_DIR_HEADERS=$(mktemp -d)
+TMP_DIR_MODULE=$(mktemp -d)
+SPATH="$(dirname "$(readlink -f "$0")")"
+
+tar -xvf $HEADERS_XZ -C $TMP_DIR_HEADERS > /dev/null
+
+cp -r $SPATH/testmod $TMP_DIR_MODULE/
+
+pushd $TMP_DIR_MODULE/testmod > /dev/null
+make -C $TMP_DIR_HEADERS M=$(pwd) modules
+popd > /dev/null
+
+rm -rf $TMP_DIR_HEADERS
+rm -rf $TMP_DIR_MODULE
diff --git a/tools/testing/selftests/kheaders/testmod/Makefile b/tools/testing/selftests/kheaders/testmod/Makefile
new file mode 100644
index 000000000000..7083e28706e8
--- /dev/null
+++ b/tools/testing/selftests/kheaders/testmod/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+
+obj-m += test.o
diff --git a/tools/testing/selftests/kheaders/testmod/test.c b/tools/testing/selftests/kheaders/testmod/test.c
new file mode 100644
index 000000000000..6eb0b8492ffa
--- /dev/null
+++ b/tools/testing/selftests/kheaders/testmod/test.c
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+
+static int __init hello_init(void)
+{
+ printk(KERN_INFO "Hello, world\n");
+ return 0;
+}
+
+static void __exit hello_exit(void)
+{
+ printk(KERN_INFO "Goodbye, world\n");
+}
+
+module_init(hello_init);
+module_exit(hello_exit);
+MODULE_LICENSE("GPL v2");
--
2.21.0.352.gf09ad66450-goog

2019-03-02 22:01:24

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Joel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc8]
[cannot apply to next-20190301]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=sh

All errors (new ones prefixed by >>):

>> find: 'arch/sh/kernel/module.lds': No such file or directory
>> find: 'arch/sh/kernel/module.lds': No such file or directory

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Attachments:

(No filename) (1.10 kB)
.config.gz (49.62 kB)
Download all attachments

2019-03-03 02:05:53

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Joel,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc8]
[cannot apply to next-20190301]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
config: microblaze-allmodconfig (attached as .config)
compiler: microblaze-linux-gcc (GCC) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=microblaze

All errors (new ones prefixed by >>):

>> find: 'arch/microblaze/kernel/module.lds': No such file or directory
>> find: 'arch/microblaze/kernel/module.lds': No such file or directory

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation

Attachments:

(No filename) (1.12 kB)
.config.gz (54.88 kB)
Download all attachments

2019-03-03 16:12:58

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

This report is for an older version of the patch so ignore it. The
issue is already resolved.

On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
>
> Hi Joel,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.0-rc8]
> [cannot apply to next-20190301]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> config: sh-allmodconfig (attached as .config)
> compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> reproduce:
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # save the attached .config to linux build tree
> GCC_VERSION=8.2.0 make.cross ARCH=sh
>
> All errors (new ones prefixed by >>):
>
> >> find: 'arch/sh/kernel/module.lds': No such file or directory
> >> find: 'arch/sh/kernel/module.lds': No such file or directory
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
>
> --
> You received this message because you are subscribed to the Google Groups "kernel-team" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

2019-03-04 14:03:20

by Qais Yousef

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On 03/01/19 11:08, Joel Fernandes (Google) wrote:
> 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. Raw kernel headers
> also cannot be copied into the filesystem like they can be on other
> distros, due to licensing and other issues. There's no linux-headers
> package on Android. 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 feature is also buildable as a module just in case the user desires
> it not being part of the kernel image. This makes it possible to load
> and unload the headers 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.
>
> 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)
> A limitation of module building with this is, 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.
>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>

I could get the headers using this patch in both built-in and modules options.

You can add my tested-and-reviewed-by: Qais Yousef <[email protected]>

I am not familiar with running kselftests so didn't get a chance to try the
next patch.

Thanks

--
Qais Yousef

> ---
>
> 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.
>
> Other notes:
> By the way I still see this error (without the patch) when doing a clean
> build: Makefile:594: include/config/auto.conf: No such file or directory
>
> It appears to be because of commit 0a16d2e8cb7e ("kbuild: use 'include'
> directive to load auto.conf from top Makefile")
>
> Documentation/dontdiff | 1 +
> init/Kconfig | 11 ++++++
> kernel/.gitignore | 3 ++
> kernel/Makefile | 37 +++++++++++++++++++
> kernel/kheaders.c | 72 ++++++++++++++++++++++++++++++++++++
> scripts/gen_ikh_data.sh | 78 +++++++++++++++++++++++++++++++++++++++
> scripts/strip-comments.pl | 8 ++++
> 7 files changed, 210 insertions(+)
> create mode 100644 kernel/kheaders.c
> create mode 100755 scripts/gen_ikh_data.sh
> create mode 100755 scripts/strip-comments.pl
>
> diff --git a/Documentation/dontdiff b/Documentation/dontdiff
> index 2228fcc8e29f..05a2319ee2a2 100644
> --- a/Documentation/dontdiff
> +++ b/Documentation/dontdiff
> @@ -151,6 +151,7 @@ int8.c
> kallsyms
> kconfig
> keywords.c
> +kheaders_data.h*
> ksym.c*
> ksym.h*
> kxgettext
> diff --git a/init/Kconfig b/init/Kconfig
> index c9386a365eea..63ff0990ae55 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -563,6 +563,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"
> + select BUILD_BIN2C
> + 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, and 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 to get access to them.
> +
> 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 b3097bde4e9c..484018945e93 100644
> --- a/kernel/.gitignore
> +++ b/kernel/.gitignore
> @@ -3,5 +3,8 @@
> #
> config_data.h
> config_data.gz
> +kheaders.md5
> +kheaders_data.h
> +kheaders_data.tar.xz
> timeconst.h
> hz.bc
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 6aa7543bcdb2..240685a6b638 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
> @@ -130,3 +131,39 @@ filechk_ikconfiggz = \
> targets += config_data.h
> $(obj)/config_data.h: $(obj)/config_data.gz FORCE
> $(call filechk,ikconfiggz)
> +
> +# 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)/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.h
> +
> +targets += 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)
> +
> +filechk_ikheadersxz = \
> + echo "static const char kernel_headers_data[] __used = KH_MAGIC_START"; \
> + cat $< | scripts/bin2c; \
> + echo "KH_MAGIC_END;"
> +
> +targets += kheaders_data.h
> +targets += kheaders.md5
> +$(obj)/kheaders_data.h: $(obj)/kheaders_data.tar.xz FORCE
> + $(call filechk,ikheadersxz)
> diff --git a/kernel/kheaders.c b/kernel/kheaders.c
> new file mode 100644
> index 000000000000..46a6358301e5
> --- /dev/null
> +++ b/kernel/kheaders.c
> @@ -0,0 +1,72 @@
> +// 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_size, which contains the
> + * compressed kernel headers. The file is first compressed with xz and then
> + * bounded by two eight byte magic numbers to allow extraction from a binary
> + * kernel image:
> + *
> + * IKHD_ST
> + * <image>
> + * IKHD_ED
> + */
> +#define KH_MAGIC_START "IKHD_ST"
> +#define KH_MAGIC_END "IKHD_ED"
> +#include "kheaders_data.h"
> +
> +
> +#define KH_MAGIC_SIZE (sizeof(KH_MAGIC_START) - 1)
> +#define kernel_headers_data_size \
> + (sizeof(kernel_headers_data) - 1 - KH_MAGIC_SIZE * 2)
> +
> +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 + KH_MAGIC_SIZE,
> + kernel_headers_data_size);
> +}
> +
> +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_size);
> +
> + 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..1fa5628fcc30
> --- /dev/null
> +++ b/scripts/gen_ikh_data.sh
> @@ -0,0 +1,78 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +
> +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
> + 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')
> + 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 -I {} sh -c "$spath/strip-comments.pl {}"
> +
> +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/scripts/strip-comments.pl b/scripts/strip-comments.pl
> new file mode 100755
> index 000000000000..f8ada87c5802
> --- /dev/null
> +++ b/scripts/strip-comments.pl
> @@ -0,0 +1,8 @@
> +#!/usr/bin/perl -pi
> +# SPDX-License-Identifier: GPL-2.0
> +
> +# This script removes /**/ comments from a file, unless such comments
> +# contain "SPDX". It is used when building compressed in-kernel headers.
> +
> +BEGIN {undef $/;}
> +s/\/\*((?!SPDX).)*?\*\///smg;
> --
> 2.21.0.352.gf09ad66450-goog

2019-03-04 22:50:53

by Dietmar Eggemann

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On 3/1/19 5:08 PM, Joel Fernandes (Google) wrote:
> 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. Raw kernel headers
> also cannot be copied into the filesystem like they can be on other
> distros, due to licensing and other issues. There's no linux-headers
> package on Android. 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 feature is also buildable as a module just in case the user desires
> it not being part of the kernel image. This makes it possible to load
> and unload the headers 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.
>
> 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)
> A limitation of module building with this is, 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.
>
> Signed-off-by: Joel Fernandes (Google) <[email protected]>
> ---
>
> 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.

Tested on x86 with eBPF scripts and exporting an alternative
BCC_KERNEL_SOURCE.

Tested-by: Dietmar Eggemann <[email protected]>

2019-03-05 17:59:05

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 04, 2019 at 11:48:52PM +0100, Dietmar Eggemann wrote:
> On 3/1/19 5:08 PM, Joel Fernandes (Google) wrote:
> > 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. Raw kernel headers
> > also cannot be copied into the filesystem like they can be on other
> > distros, due to licensing and other issues. There's no linux-headers
> > package on Android. 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 feature is also buildable as a module just in case the user desires
> > it not being part of the kernel image. This makes it possible to load
> > and unload the headers 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.
> >
> > 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)
> > A limitation of module building with this is, 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.
> >
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
> > ---
> >
> > 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.
>
> Tested on x86 with eBPF scripts and exporting an alternative
> BCC_KERNEL_SOURCE.
>
> Tested-by: Dietmar Eggemann <[email protected]>

Thank you! Will repost with the tags soon.

- Joel

2019-03-05 18:00:02

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 04, 2019 at 02:00:59PM +0000, Qais Yousef wrote:
> On 03/01/19 11:08, Joel Fernandes (Google) wrote:
> > 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. Raw kernel headers
> > also cannot be copied into the filesystem like they can be on other
> > distros, due to licensing and other issues. There's no linux-headers
> > package on Android. 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 feature is also buildable as a module just in case the user desires
> > it not being part of the kernel image. This makes it possible to load
> > and unload the headers 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.
> >
> > 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)
> > A limitation of module building with this is, 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.
> >
> > Signed-off-by: Joel Fernandes (Google) <[email protected]>
>
> I could get the headers using this patch in both built-in and modules options.
>
> You can add my tested-and-reviewed-by: Qais Yousef <[email protected]>
>
> I am not familiar with running kselftests so didn't get a chance to try the
> next patch.

Thanks a lot for testing! Will repost with the tags soon.

- Joel

2019-03-06 14:48:59

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 4, 2019 at 1:15 AM Joel Fernandes <[email protected]> wrote:
>
> This report is for an older version of the patch so ignore it. The
> issue is already resolved.
>
> On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
> >
> > Hi Joel,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on linus/master]
> > [also build test ERROR on v5.0-rc8]
> > [cannot apply to next-20190301]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> > config: sh-allmodconfig (attached as .config)
> > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> > reproduce:
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # save the attached .config to linux build tree
> > GCC_VERSION=8.2.0 make.cross ARCH=sh
> >
> > All errors (new ones prefixed by >>):
> >
> > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> >
> > ---
> > 0-DAY kernel test infrastructure Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all Intel Corporation
> >
> > --
> > You received this message because you are subscribed to the Google Groups "kernel-team" group.
> > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].

Honestly speaking, I am worried about some flaws
in this feature, but anyway I read your code.

Here are just comments from the build system point of view
in case something might be useful.

Please feel free to adopt some of them if you think they are good.

(I do not think you need to re-post a patch just for
adding Reviewed-by/Tested-by tags, but anyway looks like you are
planning to post a new version.)

[1]

Please do not ignore kbuild test robot.

It never makes such a mistake like
replying to a new patch about the test result from old version.

The reports are really addressing this version (v4).

I see the error message even on x86.

[2]

The shell script keeps running
even when an error occurs.

If any line in a shell script fails,
probably it went already wrong.

I highly recommend to add 'set -e'
at the very beginning of your shell script.

It will propagate the error to Make.

[3]

targets += kheaders_data.tar.xz
targets += kheaders_data.h
targets += kheaders.md5

These three lines are unneeded because
'targets' is necessary just for if_changed or if_changed_rule.

Instead, please add

clean-files := kheaders_data.tar.xz kheaders_data.h kheaders.md5

[4]

It is pointless to pass 'ikh_file_list'
from Makefile to the shell script.

You can directly describe the following in gen_ikh_data.sh

You do not need to use sed.

src_file_list="
include/
arch/$SRCARCH/Makefile
arch/$SRCARCH/include/
arch/$SRCARCH/kernel/module.lds
scripts/
Makefile
"

obj_file_list="
scripts/
include/
arch/$SRCARCH/include/
"

if grep -q "^CONFIG_STACK_VALIDATION=y" $KCONFIG_CONFIG; then
obj_file_list="$obj_file_list tools/objtool/objtool"
fi

Be careful about module.lds
so that it will work for all architectures.

[5]
strip-comments.pl is short enough,
and I do not assume any other user.

IMHO, it would be cleaner to embed the one-liner perl
into the shell script, for example, like follows:

find $cpio_dir -type f -print0 |
xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'

[6]
It might be better to move
scripts/gen_ikh_data.sh to kernel/gen_ikh_data.sh

It will make this feature self-contained in kernel/.
And (more importantly to me), it would reduce my maintenance field.

[7]
I do not understand for what 'tarfile_md5' is used.
Is it necessary?

[8]
Is it more efficient to pipe the header files
to perl script like this?

cat (header) | perl 'do something' > (tmp directory)

Actually, I am not sure if it is more efficient than
stripping comments in-line. I could be wrong...

[9]

I'd prefer avoiding 'pushd && popd' if possible.

Hint: Kbuild already runs at the top directory
of objtree.

It is OK if the change would mess up the script.

[10]

You can embed a binary directly into C file
without producing a giant header file.

I refactored kernel/configs.c
https://lore.kernel.org/patchwork/patch/1042013/

Be careful; my patch has not been merged yet into the mainline.

It has been a while in linux-next,
and I have not received any problem report.

So, I am guessing it will probably be merged
in the current MW.

That's all from me.

--
Best Regards
Masahiro Yamada

2019-03-06 20:13:27

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Masahiro,
Thanks for review, my replies are inline:

On Wed, Mar 06, 2019 at 09:26:14PM +0900, Masahiro Yamada wrote:
> On Mon, Mar 4, 2019 at 1:15 AM Joel Fernandes <[email protected]> wrote:
> >
> > This report is for an older version of the patch so ignore it. The
> > issue is already resolved.
> >
> > On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
> > >
> > > Hi Joel,
> > >
> > > Thank you for the patch! Yet something to improve:
> > >
> > > [auto build test ERROR on linus/master]
> > > [also build test ERROR on v5.0-rc8]
> > > [cannot apply to next-20190301]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > >
> > > url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> > > config: sh-allmodconfig (attached as .config)
> > > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> > > reproduce:
> > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > chmod +x ~/bin/make.cross
> > > # save the attached .config to linux build tree
> > > GCC_VERSION=8.2.0 make.cross ARCH=sh
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > >
> > > ---
> > > 0-DAY kernel test infrastructure Open Source Technology Center
> > > https://lists.01.org/pipermail/kbuild-all Intel Corporation
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "kernel-team" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>
>
>
> Honestly speaking, I am worried about some flaws
> in this feature, but anyway I read your code.

I am not sure what you mean by that :-(. All the previous series's comments
were addressed and it has been well tested by me and others. This looks quite
solid to me. Sorry it took so many revisions, this was not that easy to get right.

> Here are just comments from the build system point of view
> in case something might be useful.
>
> Please feel free to adopt some of them if you think they are good.
>> [1]
>
> Please do not ignore kbuild test robot.
>
> It never makes such a mistake like
> replying to a new patch about the test result from old version.
>
> The reports are really addressing this version (v4).
>
> I see the error message even on x86.

I did not mean to. By the way - the error does not cause any issues. It is
just find being noisy. You are right I missed this, and I will correct this
in v5. However, it does not cause any issues to the feature/functionality at all.

> [2]
>
> The shell script keeps running
> even when an error occurs.
>
> If any line in a shell script fails,
> probably it went already wrong.
>
> I highly recommend to add 'set -e'
> at the very beginning of your shell script.
>
> It will propagate the error to Make.

Ok I will consider making it -e. But please note that, the script itself does
not have any bugs. When you say it this way, it looks a bit bad to the
onlooker as if there is bugs in the code, but there aren't any that I know
off. All the comments in this round of review from view are just cosmetic
changes.

> [3]
>
> targets += kheaders_data.tar.xz
> targets += kheaders_data.h
> targets += kheaders.md5
>
> These three lines are unneeded because
> 'targets' is necessary just for if_changed or if_changed_rule.
>
> Instead, please add
>
> clean-files := kheaders_data.tar.xz kheaders_data.h kheaders.md5

Ok.

> [4]
>
> It is pointless to pass 'ikh_file_list'
> from Makefile to the shell script.
>
>
> You can directly describe the following in gen_ikh_data.sh
>
> You do not need to use sed.
>
>
> src_file_list="
> include/
> arch/$SRCARCH/Makefile
> arch/$SRCARCH/include/
> arch/$SRCARCH/kernel/module.lds
> scripts/
> Makefile
> "
>
> obj_file_list="
> scripts/
> include/
> arch/$SRCARCH/include/
> "

Honestly, I prefer to keep it in Makefile.

> if grep -q "^CONFIG_STACK_VALIDATION=y" $KCONFIG_CONFIG; then
> obj_file_list="$obj_file_list tools/objtool/objtool"
> fi
>
>
>
> Be careful about module.lds
> so that it will work for all architectures.

Yes.

> [5]
> strip-comments.pl is short enough,
> and I do not assume any other user.
>
>
> IMHO, it would be cleaner to embed the one-liner perl
> into the shell script, for example, like follows:
>
> find $cpio_dir -type f -print0 |
> xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'

This does not work. I already tried it. But I guess I could generate the
script on the fly.

> [6]
> It might be better to move
> scripts/gen_ikh_data.sh to kernel/gen_ikh_data.sh
>
> It will make this feature self-contained in kernel/.
> And (more importantly to me), it would reduce my maintenance field.

Sure.

> [7]
> I do not understand for what 'tarfile_md5' is used.
> Is it necessary?

It is just precaution, incase tar got corrupted and needs to be regenerated.

> [8]
> Is it more efficient to pipe the header files
> to perl script like this?
>
>
> cat (header) | perl 'do something' > (tmp directory)

I don't think so.

> [9]
>
> I'd prefer avoiding 'pushd && popd' if possible.
>
> Hint: Kbuild already runs at the top directory
> of objtree.
>
>
> It is OK if the change would mess up the script.

This is needed to make out of tree builds work.

> [10]
>
> You can embed a binary directly into C file
> without producing a giant header file.
>
> I refactored kernel/configs.c
> https://lore.kernel.org/patchwork/patch/1042013/
>
> Be careful; my patch has not been merged yet into the mainline.
>
> It has been a while in linux-next,
> and I have not received any problem report.
>
> So, I am guessing it will probably be merged
> in the current MW.

Ok I will consider doing this.

Thanks for the review!

- Joel

2019-03-06 20:31:32

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

> On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
> >
> > Hi Joel,
> >
> > Thank you for the patch! Yet something to improve:
> >
> > [auto build test ERROR on linus/master]
> > [also build test ERROR on v5.0-rc8]
> > [cannot apply to next-20190301]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> > config: sh-allmodconfig (attached as .config)
> > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> > reproduce:
> > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > chmod +x ~/bin/make.cross
> > # save the attached .config to linux build tree
> > GCC_VERSION=8.2.0 make.cross ARCH=sh
> >
> > All errors (new ones prefixed by >>):
> >
> > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > >> find: 'arch/sh/kernel/module.lds': No such file or directory
On Sun, Mar 03, 2019 at 08:11:59AM -0800, Joel Fernandes wrote:
> This report is for an older version of the patch so ignore it. The
> issue is already resolved.
>

Apologies, this issue is real. However it does not cause any failure. It is
only causing find command to be a bit noisy.

The below diff fixes it and I'll update the patch for v5:

diff --git a/scripts/gen_ikh_data.sh b/scripts/gen_ikh_data.sh
index 1fa5628fcc30..9a7ea856acbc 100755
--- a/scripts/gen_ikh_data.sh
+++ b/scripts/gen_ikh_data.sh
@@ -1,5 +1,6 @@
#!/bin/bash
# SPDX-License-Identifier: GPL-2.0
+set -e

spath="$(dirname "$(readlink -f "$0")")"
kroot="$spath/.."
@@ -11,12 +12,14 @@ 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

--
2.21.0.352.gf09ad66450-goog

2019-03-07 04:57:38

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, Mar 7, 2019 at 5:31 AM Joel Fernandes <[email protected]> wrote:
>
>
> > On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
> > >
> > > Hi Joel,
> > >
> > > Thank you for the patch! Yet something to improve:
> > >
> > > [auto build test ERROR on linus/master]
> > > [also build test ERROR on v5.0-rc8]
> > > [cannot apply to next-20190301]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > >
> > > url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> > > config: sh-allmodconfig (attached as .config)
> > > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> > > reproduce:
> > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > chmod +x ~/bin/make.cross
> > > # save the attached .config to linux build tree
> > > GCC_VERSION=8.2.0 make.cross ARCH=sh
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> On Sun, Mar 03, 2019 at 08:11:59AM -0800, Joel Fernandes wrote:
> > This report is for an older version of the patch so ignore it. The
> > issue is already resolved.
> >
>
> Apologies, this issue is real. However it does not cause any failure. It is
> only causing find command to be a bit noisy.
>
> The below diff fixes it and I'll update the patch for v5:

Let's take a look a little bit closer.

$ find arch -name module.lds
arch/ia64/module.lds
arch/powerpc/kernel/module.lds
arch/riscv/kernel/module.lds
arch/arm/kernel/module.lds
arch/m68k/kernel/module.lds
arch/arm64/kernel/module.lds

(1) module.lds may not exist for some architectures
(2) module.lds may be located in a different directory (ia64)

Your fix-up missed (2).

> diff --git a/scripts/gen_ikh_data.sh b/scripts/gen_ikh_data.sh
> index 1fa5628fcc30..9a7ea856acbc 100755
> --- a/scripts/gen_ikh_data.sh
> +++ b/scripts/gen_ikh_data.sh
> @@ -1,5 +1,6 @@
> #!/bin/bash
> # SPDX-License-Identifier: GPL-2.0
> +set -e
>
> spath="$(dirname "$(readlink -f "$0")")"
> kroot="$spath/.."
> @@ -11,12 +12,14 @@ 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
>
> --
> 2.21.0.352.gf09ad66450-goog
>

--
Best Regards
Masahiro Yamada

2019-03-07 05:02:18

by Masahiro Yamada

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, Mar 7, 2019 at 5:13 AM Joel Fernandes <[email protected]> wrote:
>
> Hi Masahiro,
> Thanks for review, my replies are inline:
>
> On Wed, Mar 06, 2019 at 09:26:14PM +0900, Masahiro Yamada wrote:
> > On Mon, Mar 4, 2019 at 1:15 AM Joel Fernandes <[email protected]> wrote:
> > >
> > > This report is for an older version of the patch so ignore it. The
> > > issue is already resolved.
> > >
> > > On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
> > > >
> > > > Hi Joel,
> > > >
> > > > Thank you for the patch! Yet something to improve:
> > > >
> > > > [auto build test ERROR on linus/master]
> > > > [also build test ERROR on v5.0-rc8]
> > > > [cannot apply to next-20190301]
> > > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > > >
> > > > url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> > > > config: sh-allmodconfig (attached as .config)
> > > > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> > > > reproduce:
> > > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > > chmod +x ~/bin/make.cross
> > > > # save the attached .config to linux build tree
> > > > GCC_VERSION=8.2.0 make.cross ARCH=sh
> > > >
> > > > All errors (new ones prefixed by >>):
> > > >
> > > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > > >
> > > > ---
> > > > 0-DAY kernel test infrastructure Open Source Technology Center
> > > > https://lists.01.org/pipermail/kbuild-all Intel Corporation
> > > >
> > > > --
> > > > You received this message because you are subscribed to the Google Groups "kernel-team" group.
> > > > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> >
> >
> >
> > Honestly speaking, I am worried about some flaws
> > in this feature, but anyway I read your code.
>
> I am not sure what you mean by that :-(. All the previous series's comments
> were addressed and it has been well tested by me and others. This looks quite
> solid to me. Sorry it took so many revisions, this was not that easy to get right.
>
> > Here are just comments from the build system point of view
> > in case something might be useful.
> >
> > Please feel free to adopt some of them if you think they are good.
> >> [1]
> >
> > Please do not ignore kbuild test robot.
> >
> > It never makes such a mistake like
> > replying to a new patch about the test result from old version.
> >
> > The reports are really addressing this version (v4).
> >
> > I see the error message even on x86.
>
> I did not mean to. By the way - the error does not cause any issues. It is
> just find being noisy. You are right I missed this, and I will correct this
> in v5. However, it does not cause any issues to the feature/functionality at all.
>
> > [2]
> >
> > The shell script keeps running
> > even when an error occurs.
> >
> > If any line in a shell script fails,
> > probably it went already wrong.
> >
> > I highly recommend to add 'set -e'
> > at the very beginning of your shell script.
> >
> > It will propagate the error to Make.
>
> Ok I will consider making it -e. But please note that, the script itself does
> not have any bugs.

Heh.
I do not know why you are so confident with your code, though.

OK, let's say this script has no bug.

But, the script may fail for a different reason.
For example, what if cpio is not installed on the user's machine.

In such a situation, this script will produce empty
kernel/kheaders_data.tar.xz, but the build system
will continue running, and succeed.

Don't you think it will better to let the build immediately fail
than letting the user debug a wrongly created image ?

> When you say it this way, it looks a bit bad to the
> onlooker as if there is bugs in the code, but there aren't any that I know
> off. All the comments in this round of review from view are just cosmetic
> changes.
>
> > [3]
> >
> > targets += kheaders_data.tar.xz
> > targets += kheaders_data.h
> > targets += kheaders.md5
> >
> > These three lines are unneeded because
> > 'targets' is necessary just for if_changed or if_changed_rule.
> >
> > Instead, please add
> >
> > clean-files := kheaders_data.tar.xz kheaders_data.h kheaders.md5
>
> Ok.
>
> > [4]
> >
> > It is pointless to pass 'ikh_file_list'
> > from Makefile to the shell script.
> >
> >
> > You can directly describe the following in gen_ikh_data.sh
> >
> > You do not need to use sed.
> >
> >
> > src_file_list="
> > include/
> > arch/$SRCARCH/Makefile
> > arch/$SRCARCH/include/
> > arch/$SRCARCH/kernel/module.lds
> > scripts/
> > Makefile
> > "
> >
> > obj_file_list="
> > scripts/
> > include/
> > arch/$SRCARCH/include/
> > "
>
> Honestly, I prefer to keep it in Makefile.
>
> > if grep -q "^CONFIG_STACK_VALIDATION=y" $KCONFIG_CONFIG; then
> > obj_file_list="$obj_file_list tools/objtool/objtool"
> > fi
> >
> >
> >
> > Be careful about module.lds
> > so that it will work for all architectures.
>
> Yes.
>
> > [5]
> > strip-comments.pl is short enough,
> > and I do not assume any other user.
> >
> >
> > IMHO, it would be cleaner to embed the one-liner perl
> > into the shell script, for example, like follows:
> >
> > find $cpio_dir -type f -print0 |
> > xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'
>
> This does not work. I already tried it. But I guess I could generate the
> script on the fly.

I tested my code, and it worked for me.

perl -e <command_line>

is useful to run short code directly.

I do not know why you say it does not work.

Masahiro

> > [6]
> > It might be better to move
> > scripts/gen_ikh_data.sh to kernel/gen_ikh_data.sh
> >
> > It will make this feature self-contained in kernel/.
> > And (more importantly to me), it would reduce my maintenance field.
>
> Sure.
>
> > [7]
> > I do not understand for what 'tarfile_md5' is used.
> > Is it necessary?
>
> It is just precaution, incase tar got corrupted and needs to be regenerated.
>
> > [8]
> > Is it more efficient to pipe the header files
> > to perl script like this?
> >
> >
> > cat (header) | perl 'do something' > (tmp directory)
>
> I don't think so.
>
> > [9]
> >
> > I'd prefer avoiding 'pushd && popd' if possible.
> >
> > Hint: Kbuild already runs at the top directory
> > of objtree.
> >
> >
> > It is OK if the change would mess up the script.
>
> This is needed to make out of tree builds work.
>
> > [10]
> >
> > You can embed a binary directly into C file
> > without producing a giant header file.
> >
> > I refactored kernel/configs.c
> > https://lore.kernel.org/patchwork/patch/1042013/
> >
> > Be careful; my patch has not been merged yet into the mainline.
> >
> > It has been a while in linux-next,
> > and I have not received any problem report.
> >
> > So, I am guessing it will probably be merged
> > in the current MW.
>
> Ok I will consider doing this.
>
> Thanks for the review!
>
> - Joel
>

2019-03-07 08:59:41

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Joel,

On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
<[email protected]> wrote:
> 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. Raw kernel headers
> also cannot be copied into the filesystem like they can be on other
> distros, due to licensing and other issues. There's no linux-headers
> package on Android. 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 feature is also buildable as a module just in case the user desires
> it not being part of the kernel image. This makes it possible to load
> and unload the headers 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.
>
> 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

As the usage pattern will be accessing the individual files, what about
implementing a file system that provides read-only access to the internal
kheaders archive?

mount kheaders $HOME/headers -t kheaders

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-03-07 14:55:43

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, Mar 07, 2019 at 01:59:12PM +0900, Masahiro Yamada wrote:
[snip]
> > > [2]
> > >
> > > The shell script keeps running
> > > even when an error occurs.
> > >
> > > If any line in a shell script fails,
> > > probably it went already wrong.
> > >
> > > I highly recommend to add 'set -e'
> > > at the very beginning of your shell script.
> > >
> > > It will propagate the error to Make.
> >
> > Ok I will consider making it -e. But please note that, the script itself does
> > not have any bugs.
>
> Heh.
> I do not know why you are so confident with your code, though.
>
> OK, let's say this script has no bug.
>
> But, the script may fail for a different reason.
> For example, what if cpio is not installed on the user's machine.
>
> In such a situation, this script will produce empty
> kernel/kheaders_data.tar.xz, but the build system
> will continue running, and succeed.
>
> Don't you think it will better to let the build immediately fail
> than letting the user debug a wrongly created image ?

Yes, I do think it is better and I am doing that in the next revision
already. It is appended below.

> > > [5]
> > > strip-comments.pl is short enough,
> > > and I do not assume any other user.
> > >
> > >
> > > IMHO, it would be cleaner to embed the one-liner perl
> > > into the shell script, for example, like follows:
> > >
> > > find $cpio_dir -type f -print0 |
> > > xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'
> >
> > This does not work. I already tried it. But I guess I could generate the
> > script on the fly.
>
>
> I tested my code, and it worked for me.
>
> perl -e <command_line>
>
> is useful to run short code directly.
>

Sorry, previous attempts to use perl -pie didn't work but I tried your one
liner and it does work. Below is the updated patch with this and all the
other nits addressed including the module.lds one.

About module.lds for ia64 architecture, sorry to miss that - I did not test
all architectures, just arm64 and x86 which I have hardware for.
Interestingly kbuild robot didn't catch it either. Anyway it is addressed below.

Thanks a lot for all the great review.

---8<-----------------------

From 22cfd523f14c3b7e13fa05fc1c483a4d368e00d1 Mon Sep 17 00:00:00 2001
From: "Joel Fernandes (Google)" <[email protected]>
Date: Fri, 18 Jan 2019 15:52:41 -0500
Subject: [PATCH v4.1] Provide in-kernel headers for making it easy to extend the
kernel

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. Raw kernel headers
also cannot be copied into the filesystem like they can be on other
distros, due to licensing and other issues. There's no linux-headers
package on Android. 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 feature is also buildable as a module just in case the user desires
it not being part of the kernel image. This makes it possible to load
and unload the headers 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.

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)
A limitation of module building with this is, 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.

Tested-by: [email protected]
Tested-by: [email protected]
Tested-by: [email protected]
Signed-off-by: Joel Fernandes (Google) <[email protected]>
diff-note-start

Changes since v4:
- 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.
- added Tested-by tags from ARM folks.
- TODO: several more Masahiro comments.

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) <[email protected]>
---
Documentation/dontdiff | 1 +
init/Kconfig | 11 ++++++
kernel/.gitignore | 3 ++
kernel/Makefile | 36 ++++++++++++++++++
kernel/kheaders.c | 72 ++++++++++++++++++++++++++++++++++++
scripts/gen_ikh_data.sh | 81 +++++++++++++++++++++++++++++++++++++++++
6 files changed, 204 insertions(+)
create mode 100644 kernel/kheaders.c
create mode 100755 scripts/gen_ikh_data.sh

diff --git a/Documentation/dontdiff b/Documentation/dontdiff
index 2228fcc8e29f..05a2319ee2a2 100644
--- a/Documentation/dontdiff
+++ b/Documentation/dontdiff
@@ -151,6 +151,7 @@ int8.c
kallsyms
kconfig
keywords.c
+kheaders_data.h*
ksym.c*
ksym.h*
kxgettext
diff --git a/init/Kconfig b/init/Kconfig
index c9386a365eea..63ff0990ae55 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -563,6 +563,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"
+ select BUILD_BIN2C
+ 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, and 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 to get access to them.
+
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 b3097bde4e9c..484018945e93 100644
--- a/kernel/.gitignore
+++ b/kernel/.gitignore
@@ -3,5 +3,8 @@
#
config_data.h
config_data.gz
+kheaders.md5
+kheaders_data.h
+kheaders_data.tar.xz
timeconst.h
hz.bc
diff --git a/kernel/Makefile b/kernel/Makefile
index 6aa7543bcdb2..2c7f5e9a2e9f 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
@@ -130,3 +131,38 @@ filechk_ikconfiggz = \
targets += config_data.h
$(obj)/config_data.h: $(obj)/config_data.gz FORCE
$(call filechk,ikconfiggz)
+
+# 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.h
+
+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)
+
+filechk_ikheadersxz = \
+ echo "static const char kernel_headers_data[] __used = KH_MAGIC_START"; \
+ cat $< | scripts/bin2c; \
+ echo "KH_MAGIC_END;"
+
+$(obj)/kheaders_data.h: $(obj)/kheaders_data.tar.xz FORCE
+ $(call filechk,ikheadersxz)
+
+clean-files := kheaders_data.tar.xz kheaders_data.h kheaders.md5
diff --git a/kernel/kheaders.c b/kernel/kheaders.c
new file mode 100644
index 000000000000..46a6358301e5
--- /dev/null
+++ b/kernel/kheaders.c
@@ -0,0 +1,72 @@
+// 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_size, which contains the
+ * compressed kernel headers. The file is first compressed with xz and then
+ * bounded by two eight byte magic numbers to allow extraction from a binary
+ * kernel image:
+ *
+ * IKHD_ST
+ * <image>
+ * IKHD_ED
+ */
+#define KH_MAGIC_START "IKHD_ST"
+#define KH_MAGIC_END "IKHD_ED"
+#include "kheaders_data.h"
+
+
+#define KH_MAGIC_SIZE (sizeof(KH_MAGIC_START) - 1)
+#define kernel_headers_data_size \
+ (sizeof(kernel_headers_data) - 1 - KH_MAGIC_SIZE * 2)
+
+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 + KH_MAGIC_SIZE,
+ kernel_headers_data_size);
+}
+
+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_size);
+
+ 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..bcd694e9a105
--- /dev/null
+++ b/scripts/gen_ikh_data.sh
@@ -0,0 +1,81 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+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.352.gf09ad66450-goog

2019-03-07 15:04:44

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, Mar 07, 2019 at 09:58:24AM +0100, Geert Uytterhoeven wrote:
> Hi Joel,
>
> On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
> <[email protected]> wrote:
> > 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. Raw kernel headers
> > also cannot be copied into the filesystem like they can be on other
> > distros, due to licensing and other issues. There's no linux-headers
> > package on Android. 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 feature is also buildable as a module just in case the user desires
> > it not being part of the kernel image. This makes it possible to load
> > and unload the headers 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.
> >
> > 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
>
> As the usage pattern will be accessing the individual files, what about
> implementing a file system that provides read-only access to the internal
> kheaders archive?
>
> mount kheaders $HOME/headers -t kheaders

I thought about it already. This is easier said than done though. The archive
is compressed from 40MB to 3.6MB. If we leave it uncompressed in RAM, then it
will take up the entire 40MB of RAM and in Android we don't even use
disk-based swap.

So we will need some kind of intra file compressed memory representation that
a filesystem can use for the backing store. I thought of RAM-backed squashfs
but it requires squashfs-tools to be installed at build time (which my host
distro itself didn't have).

It is just so much easier to use tar + xz at build time, and leave the
decompression task to the user. After decompression, the files will live on
the disk and the page-cache mechanism will free memory when/if the files fall
off the LRUs.

WDYT?

thanks,

- Joel

2019-03-07 15:24:01

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, Mar 07, 2019 at 10:03:43AM -0500, Joel Fernandes wrote:
> On Thu, Mar 07, 2019 at 09:58:24AM +0100, Geert Uytterhoeven wrote:
> > Hi Joel,
> >
> > On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
> > <[email protected]> wrote:
> > > 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. Raw kernel headers
> > > also cannot be copied into the filesystem like they can be on other
> > > distros, due to licensing and other issues. There's no linux-headers
> > > package on Android. 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 feature is also buildable as a module just in case the user desires
> > > it not being part of the kernel image. This makes it possible to load
> > > and unload the headers 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.
> > >
> > > 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
> >
> > As the usage pattern will be accessing the individual files, what about
> > implementing a file system that provides read-only access to the internal
> > kheaders archive?
> >
> > mount kheaders $HOME/headers -t kheaders
>
> I thought about it already. This is easier said than done though. The archive
> is compressed from 40MB to 3.6MB. If we leave it uncompressed in RAM, then it
> will take up the entire 40MB of RAM and in Android we don't even use
> disk-based swap.
>
> So we will need some kind of intra file compressed memory representation that
> a filesystem can use for the backing store. I thought of RAM-backed squashfs
> but it requires squashfs-tools to be installed at build time (which my host
> distro itself didn't have).
>
> It is just so much easier to use tar + xz at build time, and leave the
> decompression task to the user. After decompression, the files will live on
> the disk and the page-cache mechanism will free memory when/if the files fall
> off the LRUs.
>
> WDYT?

I think the compressed tarball is much simpler/easier overall. If
someone really wants the filesystem, they just uncompress it into a
tmpfs mount. It's much less moving kernel code to worry about.

thanks,

greg k-h

2019-03-07 16:56:43

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, Mar 07, 2019 at 04:23:03PM +0100, Greg KH wrote:
> > > On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
> > > <[email protected]> wrote:
> > > > 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. Raw kernel headers
> > > > also cannot be copied into the filesystem like they can be on other
> > > > distros, due to licensing and other issues. There's no linux-headers
> > > > package on Android. 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 feature is also buildable as a module just in case the user desires
> > > > it not being part of the kernel image. This makes it possible to load
> > > > and unload the headers 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.
> > > >
> > > > 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
> > >
> > > As the usage pattern will be accessing the individual files, what about
> > > implementing a file system that provides read-only access to the internal
> > > kheaders archive?
> > >
> > > mount kheaders $HOME/headers -t kheaders
> >
> > I thought about it already. This is easier said than done though. The archive
> > is compressed from 40MB to 3.6MB. If we leave it uncompressed in RAM, then it
> > will take up the entire 40MB of RAM and in Android we don't even use
> > disk-based swap.
> >
> > So we will need some kind of intra file compressed memory representation that
> > a filesystem can use for the backing store. I thought of RAM-backed squashfs
> > but it requires squashfs-tools to be installed at build time (which my host
> > distro itself didn't have).
> >
> > It is just so much easier to use tar + xz at build time, and leave the
> > decompression task to the user. After decompression, the files will live on
> > the disk and the page-cache mechanism will free memory when/if the files fall
> > off the LRUs.
> >
> > WDYT?
>
> I think the compressed tarball is much simpler/easier overall. If
> someone really wants the filesystem, they just uncompress it into a
> tmpfs mount. It's much less moving kernel code to worry about.

Agreed, I also feel the same. thanks,

- Joel

2019-03-07 23:24:45

by Justin Capella

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Would you consider this more reliable than the make install_headers,
and am curious about DKMS applications / whether this should be
preferable to the /lib/modules/kernver/biuld symlinks

On Wed, Mar 6, 2019 at 6:48 AM Masahiro Yamada
<[email protected]> wrote:
>
> On Mon, Mar 4, 2019 at 1:15 AM Joel Fernandes <[email protected]> wrote:
> >
> > This report is for an older version of the patch so ignore it. The
> > issue is already resolved.
> >
> > On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <[email protected]> wrote:
> > >
> > > Hi Joel,
> > >
> > > Thank you for the patch! Yet something to improve:
> > >
> > > [auto build test ERROR on linus/master]
> > > [also build test ERROR on v5.0-rc8]
> > > [cannot apply to next-20190301]
> > > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> > >
> > > url: https://github.com/0day-ci/linux/commits/Joel-Fernandes-Google/Provide-in-kernel-headers-for-making-it-easy-to-extend-the-kernel/20190303-014850
> > > config: sh-allmodconfig (attached as .config)
> > > compiler: sh4-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
> > > reproduce:
> > > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> > > chmod +x ~/bin/make.cross
> > > # save the attached .config to linux build tree
> > > GCC_VERSION=8.2.0 make.cross ARCH=sh
> > >
> > > All errors (new ones prefixed by >>):
> > >
> > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > > >> find: 'arch/sh/kernel/module.lds': No such file or directory
> > >
> > > ---
> > > 0-DAY kernel test infrastructure Open Source Technology Center
> > > https://lists.01.org/pipermail/kbuild-all Intel Corporation
> > >
> > > --
> > > You received this message because you are subscribed to the Google Groups "kernel-team" group.
> > > To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
>
>
>
> Honestly speaking, I am worried about some flaws
> in this feature, but anyway I read your code.
>
> Here are just comments from the build system point of view
> in case something might be useful.
>
> Please feel free to adopt some of them if you think they are good.
>
> (I do not think you need to re-post a patch just for
> adding Reviewed-by/Tested-by tags, but anyway looks like you are
> planning to post a new version.)
>
>
>
> [1]
>
> Please do not ignore kbuild test robot.
>
> It never makes such a mistake like
> replying to a new patch about the test result from old version.
>
> The reports are really addressing this version (v4).
>
> I see the error message even on x86.
>
>
> [2]
>
> The shell script keeps running
> even when an error occurs.
>
> If any line in a shell script fails,
> probably it went already wrong.
>
> I highly recommend to add 'set -e'
> at the very beginning of your shell script.
>
> It will propagate the error to Make.
>
>
>
> [3]
>
> targets += kheaders_data.tar.xz
> targets += kheaders_data.h
> targets += kheaders.md5
>
> These three lines are unneeded because
> 'targets' is necessary just for if_changed or if_changed_rule.
>
> Instead, please add
>
> clean-files := kheaders_data.tar.xz kheaders_data.h kheaders.md5
>
>
> [4]
>
> It is pointless to pass 'ikh_file_list'
> from Makefile to the shell script.
>
>
> You can directly describe the following in gen_ikh_data.sh
>
> You do not need to use sed.
>
>
> src_file_list="
> include/
> arch/$SRCARCH/Makefile
> arch/$SRCARCH/include/
> arch/$SRCARCH/kernel/module.lds
> scripts/
> Makefile
> "
>
> obj_file_list="
> scripts/
> include/
> arch/$SRCARCH/include/
> "
>
> if grep -q "^CONFIG_STACK_VALIDATION=y" $KCONFIG_CONFIG; then
> obj_file_list="$obj_file_list tools/objtool/objtool"
> fi
>
>
>
> Be careful about module.lds
> so that it will work for all architectures.
>
>
>
>
> [5]
> strip-comments.pl is short enough,
> and I do not assume any other user.
>
>
> IMHO, it would be cleaner to embed the one-liner perl
> into the shell script, for example, like follows:
>
> find $cpio_dir -type f -print0 |
> xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;'
>
>
>
> [6]
> It might be better to move
> scripts/gen_ikh_data.sh to kernel/gen_ikh_data.sh
>
> It will make this feature self-contained in kernel/.
> And (more importantly to me), it would reduce my maintenance field.
>
>
>
> [7]
> I do not understand for what 'tarfile_md5' is used.
> Is it necessary?
>
>
>
> [8]
> Is it more efficient to pipe the header files
> to perl script like this?
>
>
> cat (header) | perl 'do something' > (tmp directory)
>
>
> Actually, I am not sure if it is more efficient than
> stripping comments in-line. I could be wrong...
>
>
> [9]
>
> I'd prefer avoiding 'pushd && popd' if possible.
>
> Hint: Kbuild already runs at the top directory
> of objtree.
>
>
> It is OK if the change would mess up the script.
>
>
> [10]
>
> You can embed a binary directly into C file
> without producing a giant header file.
>
> I refactored kernel/configs.c
> https://lore.kernel.org/patchwork/patch/1042013/
>
> Be careful; my patch has not been merged yet into the mainline.
>
> It has been a while in linux-next,
> and I have not received any problem report.
>
> So, I am guessing it will probably be merged
> in the current MW.
>
>
>
>
> That's all from me.
>
>
> --
> Best Regards
> Masahiro Yamada

2019-03-08 08:54:15

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Joel,

On Thu, Mar 7, 2019 at 4:03 PM Joel Fernandes <[email protected]> wrote:
> On Thu, Mar 07, 2019 at 09:58:24AM +0100, Geert Uytterhoeven wrote:
> > On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
> > <[email protected]> wrote:
> > > 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. Raw kernel headers
> > > also cannot be copied into the filesystem like they can be on other
> > > distros, due to licensing and other issues. There's no linux-headers
> > > package on Android. 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 feature is also buildable as a module just in case the user desires
> > > it not being part of the kernel image. This makes it possible to load
> > > and unload the headers 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.
> > >
> > > 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
> >
> > As the usage pattern will be accessing the individual files, what about
> > implementing a file system that provides read-only access to the internal
> > kheaders archive?
> >
> > mount kheaders $HOME/headers -t kheaders
>
> I thought about it already. This is easier said than done though. The archive
> is compressed from 40MB to 3.6MB. If we leave it uncompressed in RAM, then it
> will take up the entire 40MB of RAM and in Android we don't even use
> disk-based swap.

Sure.

> So we will need some kind of intra file compressed memory representation that
> a filesystem can use for the backing store. I thought of RAM-backed squashfs

Right, I didn't think of squashfs. Having a kernel module that sets up an MTD
that can be mounted with squashfs is IMHO an even better (and more generic)
solution.

> but it requires squashfs-tools to be installed at build time (which my host
> distro itself didn't have).

Squashfs not being part of your host distro is not the hardest problem
to solve...

> It is just so much easier to use tar + xz at build time, and leave the
> decompression task to the user. After decompression, the files will live on
> the disk and the page-cache mechanism will free memory when/if the files fall
> off the LRUs.

I'm also considering how generic and extensible the solution is.
What if people need other build artifacts in the future (e.g. signing key to
load signed modules)?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-03-08 13:43:28

by Enrico Weigelt, metux IT consult

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

HI Geert,

On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <[email protected]> wrote:
> > It is just so much easier to use tar + xz at build time, and leave the
> > decompression task to the user. After decompression, the files will live on
> > the disk and the page-cache mechanism will free memory when/if the files fall
> > off the LRUs.
>
> I'm also considering how generic and extensible the solution is.
> What if people need other build artifacts in the future (e.g. signing key to
> load signed modules)?

That sounds like it could be useful. I don't see any reason off the
top why that would not be possible to add to the list of archived
files in the future. The patch allows populating the list of files
from Kbuild using ikh_file_list variable.

thanks,

- Joel

2019-03-08 14:00:29

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On 08.03.19 14:42, Joel Fernandes wrote:

Hi folks,

> That sounds like it could be useful. I don't see any reason off the
> top why that would not be possible to add to the list of archived
> files in the future. The patch allows populating the list of files
> from Kbuild using ikh_file_list variable.

It seems the whole thing's going into a direction where a whole own
subtopic is coming up: compressed built-in filesystem.

Haven't had a deeper thought about it yet, whether or not existing
filesystems like squashfs, initramfs, etc are the right thing for that,
or something new should be invented, but a generic mechanism for
compiled-in compressed (ro) filesystems could also be interesting
for very small devices, that perhaps even dont need any persistence.

Some requirements coming up in mind:

1. it shall be possible to have any number of instances - possibly by
separate modules.
2. it shall be possible to use an bootloader/firmware provided image
(so it can serve as initrd)
2. data should stay compressed as long as possible, but uncompressed
data might be cached - do decompression on-demand
3. only needs to be ro (write access could be done via unionfs+friends)
4. it shall be swappable (if swap is enabled)

In that scenario, these in-kernel headers would just one consumer,
I can imagine lots of others.

--mtx

--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
[email protected] -- +49-151-27565287

2019-03-08 17:05:39

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> HI Geert,
>
> On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <[email protected]> wrote:
> > > It is just so much easier to use tar + xz at build time, and leave the
> > > decompression task to the user. After decompression, the files will live on
> > > the disk and the page-cache mechanism will free memory when/if the files fall
> > > off the LRUs.
> >
> > I'm also considering how generic and extensible the solution is.
> > What if people need other build artifacts in the future (e.g. signing key to
> > load signed modules)?
>
> That sounds like it could be useful. I don't see any reason off the
> top why that would not be possible to add to the list of archived
> files in the future. The patch allows populating the list of files
> from Kbuild using ikh_file_list variable.

Um, no, you don't want the signing key in the kernel itself, as that
totally defeats the purpose of the signing key :)

greg k-h

2019-03-08 17:24:44

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Fri, Mar 08, 2019 at 02:57:24PM +0100, Enrico Weigelt, metux IT consult wrote:
> On 08.03.19 14:42, Joel Fernandes wrote:
>
> Hi folks,
>
> > That sounds like it could be useful. I don't see any reason off the
> > top why that would not be possible to add to the list of archived
> > files in the future. The patch allows populating the list of files
> > from Kbuild using ikh_file_list variable.
>
> It seems the whole thing's going into a direction where a whole own
> subtopic is coming up: compressed built-in filesystem.
>
> Haven't had a deeper thought about it yet, whether or not existing
> filesystems like squashfs, initramfs, etc are the right thing for that,
> or something new should be invented, but a generic mechanism for
> compiled-in compressed (ro) filesystems could also be interesting
> for very small devices, that perhaps even dont need any persistence.
>
> Some requirements coming up in mind:
>
> 1. it shall be possible to have any number of instances - possibly by
> separate modules.
> 2. it shall be possible to use an bootloader/firmware provided image
> (so it can serve as initrd)
> 2. data should stay compressed as long as possible, but uncompressed
> data might be cached - do decompression on-demand
> 3. only needs to be ro (write access could be done via unionfs+friends)
> 4. it shall be swappable (if swap is enabled)
>
> In that scenario, these in-kernel headers would just one consumer,
> I can imagine lots of others.

I too want a pony :)

Until then, I think this feature should not be bikeshedded to death. It
fits a real need, is simple (modulo the Kbuild interactions), and is
optional for those that do not wish to waste the memory.

thanks,

greg k-h

2019-03-08 18:00:03

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Fri, Mar 08, 2019 at 03:02:51PM +0100, Greg KH wrote:
> On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > HI Geert,
> >
> > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > It is just so much easier to use tar + xz at build time, and leave the
> > > > decompression task to the user. After decompression, the files will live on
> > > > the disk and the page-cache mechanism will free memory when/if the files fall
> > > > off the LRUs.
> > >
> > > I'm also considering how generic and extensible the solution is.
> > > What if people need other build artifacts in the future (e.g. signing key to
> > > load signed modules)?
> >
> > That sounds like it could be useful. I don't see any reason off the
> > top why that would not be possible to add to the list of archived
> > files in the future. The patch allows populating the list of files
> > from Kbuild using ikh_file_list variable.
>
> Um, no, you don't want the signing key in the kernel itself, as that
> totally defeats the purpose of the signing key :)

*Bangs self to desk*. You are right, didn't think that one through ;)

By the way, my v5 is ready to send. But I am avoiding sending it due to the
merge window going on. I posted it yesterday in this thread as well:
https://lore.kernel.org/patchwork/comment/1241077/

This is likely to be the final version or so, unless Masahiro or you or other
reviewers have any more comments.

thanks,

- Joel

2019-03-08 18:02:10

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Greg,

On Fri, Mar 8, 2019 at 6:05 PM Greg KH <[email protected]> wrote:
> On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > It is just so much easier to use tar + xz at build time, and leave the
> > > > decompression task to the user. After decompression, the files will live on
> > > > the disk and the page-cache mechanism will free memory when/if the files fall
> > > > off the LRUs.
> > >
> > > I'm also considering how generic and extensible the solution is.
> > > What if people need other build artifacts in the future (e.g. signing key to
> > > load signed modules)?
> >
> > That sounds like it could be useful. I don't see any reason off the
> > top why that would not be possible to add to the list of archived
> > files in the future. The patch allows populating the list of files
> > from Kbuild using ikh_file_list variable.
>
> Um, no, you don't want the signing key in the kernel itself, as that
> totally defeats the purpose of the signing key :)

In a loadable module?
He who has the module, can build and sign more modules.

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-03-09 07:17:48

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Fri, Mar 08, 2019 at 06:59:23PM +0100, Geert Uytterhoeven wrote:
> Hi Greg,
>
> On Fri, Mar 8, 2019 at 6:05 PM Greg KH <[email protected]> wrote:
> > On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > > It is just so much easier to use tar + xz at build time, and leave the
> > > > > decompression task to the user. After decompression, the files will live on
> > > > > the disk and the page-cache mechanism will free memory when/if the files fall
> > > > > off the LRUs.
> > > >
> > > > I'm also considering how generic and extensible the solution is.
> > > > What if people need other build artifacts in the future (e.g. signing key to
> > > > load signed modules)?
> > >
> > > That sounds like it could be useful. I don't see any reason off the
> > > top why that would not be possible to add to the list of archived
> > > files in the future. The patch allows populating the list of files
> > > from Kbuild using ikh_file_list variable.
> >
> > Um, no, you don't want the signing key in the kernel itself, as that
> > totally defeats the purpose of the signing key :)
>
> In a loadable module?
> He who has the module, can build and sign more modules.

Again, that's pretty foolish.

Signing keys should be kept secure, or better yet, just deleted entirely
after creating and signing with them. That's what I do for my kernels
and I'm pretty sure that some distros also do this. That way there's no
chance that someone else can sign a module and have it loaded without
detection, which is what signing is supposed to prevent from happening.

thanks,

greg k-h

2019-03-09 11:42:03

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Greg,

On Sat, Mar 9, 2019 at 8:16 AM Greg KH <[email protected]> wrote:
> On Fri, Mar 08, 2019 at 06:59:23PM +0100, Geert Uytterhoeven wrote:
> > On Fri, Mar 8, 2019 at 6:05 PM Greg KH <[email protected]> wrote:
> > > On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > > > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <[email protected]> wrote:
> > > > > > It is just so much easier to use tar + xz at build time, and leave the
> > > > > > decompression task to the user. After decompression, the files will live on
> > > > > > the disk and the page-cache mechanism will free memory when/if the files fall
> > > > > > off the LRUs.
> > > > >
> > > > > I'm also considering how generic and extensible the solution is.
> > > > > What if people need other build artifacts in the future (e.g. signing key to
> > > > > load signed modules)?
> > > >
> > > > That sounds like it could be useful. I don't see any reason off the
> > > > top why that would not be possible to add to the list of archived
> > > > files in the future. The patch allows populating the list of files
> > > > from Kbuild using ikh_file_list variable.
> > >
> > > Um, no, you don't want the signing key in the kernel itself, as that
> > > totally defeats the purpose of the signing key :)
> >
> > In a loadable module?
> > He who has the module, can build and sign more modules.
>
> Again, that's pretty foolish.
>
> Signing keys should be kept secure, or better yet, just deleted entirely
> after creating and signing with them. That's what I do for my kernels
> and I'm pretty sure that some distros also do this. That way there's no
> chance that someone else can sign a module and have it loaded without
> detection, which is what signing is supposed to prevent from happening.

If you want that kind of security, there's no point in allowing to extend the
kernel by building more kernel modules after deployment.

The more I think about this (embedding a kernel headers archive in the
kernel or a kernel module), the more I feel this is a workaround for a distro
issue.
Files are distributed with the kernel image, e.g. loadable kernel modules,
so distributing more files is not a technical issue (and if it is, working
around that might be as simple as "mv kheaders.tar.xz kheaders.ko",
and letting module install take care of it ;-)
It makes sense to provide /proc/kconfig.gz, as this is unique
configuration info. The kernel headers can easily by derived from this
config, and the kernel sources (which are GPL).

"Raw kernel headers also cannot be copied into the filesystem like they
can be on other distros, due to licensing and other issues. There's no
linux-headers package on Android."

What's the licensing issue? What's the (legal) difference between having
the headers on the file system, and having a kernel module including the
headers on the file system?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-03-09 12:13:37

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Sat, Mar 09, 2019 at 12:40:01PM +0100, Geert Uytterhoeven wrote:
> > Signing keys should be kept secure, or better yet, just deleted entirely
> > after creating and signing with them. That's what I do for my kernels
> > and I'm pretty sure that some distros also do this. That way there's no
> > chance that someone else can sign a module and have it loaded without
> > detection, which is what signing is supposed to prevent from happening.
>
> If you want that kind of security, there's no point in allowing to extend the
> kernel by building more kernel modules after deployment.

That's not what these files are for (in the original user's case). They
want these for doing tracing/ebpf stuff, which require kernel headers to
build against.

> "Raw kernel headers also cannot be copied into the filesystem like they
> can be on other distros, due to licensing and other issues. There's no
> linux-headers package on Android."
>
> What's the licensing issue? What's the (legal) difference between having
> the headers on the file system, and having a kernel module including the
> headers on the file system?

There is no licensing issue, see my follow-up comment about that.

It's all in ease-of-use here. You want to build a trace function
against a running kernel, and now you have the header files for that
specific kernel right there in the kernel itself to build against. It
doesn't get easier than that.

thanks,

greg k-h

2019-03-09 16:53:51

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On 3/9/19 7:11 AM, Greg KH wrote:
> There is no licensing issue, see my follow-up comment about that.
>
> It's all in ease-of-use here. You want to build a trace function
> against a running kernel, and now you have the header files for that
> specific kernel right there in the kernel itself to build against. It
> doesn't get easier than that.

Agreed.

It seems that opinions in this thread are split along two conceptions of
a "Linux system". On one side there's that of the conventional "Linux
distro" where the entity generating the distro has full control over the
entire process and the parts thereof. On the other side there's the
world that has evolved out of the multi-party ecosystem that Android
fostered. In the later, there isn't a single party that controls all
aspects of the "distro". Instead, there are a multitude of parties
contributing to creating a fully-functional Linux-based system.

In the latter ecosystem, the kernel and the filesystems (*plural*) used
with it do not necessarily come from the same place, are maintained by
the same parties or even required to be in lock-step. For all I know,
the filesystem images are coming from one party and the kernel is at one
point from one party and at another point can be substituted, possibly
for testing purposes, without userspace images ever changing. No
licensing issues at all involved, either with regards to the headers or
the distribution of the kernel itself, which, in as far as I've seen
when speaking of known industry players (those that matter here), are
always ultimately available in source from somewhere.

Instead the issue is that I want to be able to give user-space access to
the headers that were used to build the kernel that they're running
over, regardless of where this kernel came from. And since I have to
assume that those two parts (kernel vs. user-space) are coming from
separate parties and can be arbitrarily replaced, there needs to be some
form of "contract" between them. The kernel's ABI already provides quite
a bit of that and, as Joel pointed out, Google can enforce a few more
things, such as default kernel configs, to "make things work". But one
option that doesn't exist in the world Android evolves in is to somehow
ensure that the filesystem images somehow have kernel-specific
tool-required artifacts for every possible kernel they may run against.
That's just not possible.

That's especially true in the case of modern-day Android where the final
system is made of at least half a dozen filesystem images with each
image possibly coming from a different party. The /vendor partition
(with the hardware enablement), for example, could be coming from the
device vendor while the /system partition (with the Android framework),
for example, could be coming straight from Google in the form of
"Generic System Image" -- an image meant to run as-is on any
Treble-compliant system. /system might have the tools for eBPF, but
/vendor might have modules, while still the "boot" partition might have
the kernel. There's no reason I can't replace the boot partition
*without* changing /vendor. And there's no reason I can't replace
/vendor *without* replacing the boot partition. And all other
combinations with all other images.

That, in my view, is a big part of the problem Joel's patch solves: in a
system whose functionality requires multiple *independent* parties to
work together, I can still get the necessary kernel headers for
user-space tools to properly operate regardless of which part of the
system id being substituted or replaced.

Cheers,

--
Karim Yaghmour
CEO - Opersys inc. / http://www.opersys.com
http://twitter.com/karimyaghmour

2019-03-09 19:28:41

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Karim,

Thanks for the explanation!

On Sat, Mar 9, 2019 at 5:52 PM Karim Yaghmour
<[email protected]> wrote:
> On 3/9/19 7:11 AM, Greg KH wrote:
> > There is no licensing issue, see my follow-up comment about that.
> >
> > It's all in ease-of-use here. You want to build a trace function
> > against a running kernel, and now you have the header files for that
> > specific kernel right there in the kernel itself to build against. It
> > doesn't get easier than that.
>
> Agreed.
>
> It seems that opinions in this thread are split along two conceptions of
> a "Linux system". On one side there's that of the conventional "Linux
> distro" where the entity generating the distro has full control over the
> entire process and the parts thereof. On the other side there's the
> world that has evolved out of the multi-party ecosystem that Android
> fostered. In the later, there isn't a single party that controls all
> aspects of the "distro". Instead, there are a multitude of parties
> contributing to creating a fully-functional Linux-based system.
>
> In the latter ecosystem, the kernel and the filesystems (*plural*) used
> with it do not necessarily come from the same place, are maintained by
> the same parties or even required to be in lock-step. For all I know,
> the filesystem images are coming from one party and the kernel is at one
> point from one party and at another point can be substituted, possibly
> for testing purposes, without userspace images ever changing. No
> licensing issues at all involved, either with regards to the headers or
> the distribution of the kernel itself, which, in as far as I've seen
> when speaking of known industry players (those that matter here), are
> always ultimately available in source from somewhere.
>
> Instead the issue is that I want to be able to give user-space access to
> the headers that were used to build the kernel that they're running
> over, regardless of where this kernel came from. And since I have to
> assume that those two parts (kernel vs. user-space) are coming from
> separate parties and can be arbitrarily replaced, there needs to be some
> form of "contract" between them. The kernel's ABI already provides quite
> a bit of that and, as Joel pointed out, Google can enforce a few more
> things, such as default kernel configs, to "make things work". But one
> option that doesn't exist in the world Android evolves in is to somehow
> ensure that the filesystem images somehow have kernel-specific
> tool-required artifacts for every possible kernel they may run against.
> That's just not possible.
>
> That's especially true in the case of modern-day Android where the final
> system is made of at least half a dozen filesystem images with each
> image possibly coming from a different party. The /vendor partition
> (with the hardware enablement), for example, could be coming from the
> device vendor while the /system partition (with the Android framework),
> for example, could be coming straight from Google in the form of
> "Generic System Image" -- an image meant to run as-is on any
> Treble-compliant system. /system might have the tools for eBPF, but
> /vendor might have modules, while still the "boot" partition might have
> the kernel. There's no reason I can't replace the boot partition
> *without* changing /vendor. And there's no reason I can't replace
> /vendor *without* replacing the boot partition. And all other
> combinations with all other images.

So how does this work, with kernel images and kernel modules supplied
by separate parties, not "bound" by the same kernel headers/API, as they
can be replaced separately?

> That, in my view, is a big part of the problem Joel's patch solves: in a
> system whose functionality requires multiple *independent* parties to
> work together, I can still get the necessary kernel headers for
> user-space tools to properly operate regardless of which part of the
> system id being substituted or replaced.

Isn't the need for kernel headers for user-space tools something different,
as this is limited to the uapi versions, which are less (almost not) subject
to change, compared to the kernel headers needed for compiling kernel
modules?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-03-09 21:46:22

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Geert,

On 3/9/19 2:26 PM, Geert Uytterhoeven wrote:
> Thanks for the explanation!

Happy this was useful :)

> So how does this work, with kernel images and kernel modules supplied
> by separate parties, not "bound" by the same kernel headers/API, as they
> can be replaced separately?

The thing with Android is that there isn't a "one size fits all". Google
provides guidance, policies and at least one example implementation
through the Pixel lines.

Each vendor however is allowed a great degree of latitude with regards
to what they decide to do. Personally, if I was advising a team working
on an Android device where Joel's patch was available as part of their
kernel I would just recommend that they build it in -- i.e. not as a
module. Hence, there would be no module chasing game.

With regards to Google's guidelines for manufacturers, though, Google
states that CONFIG_MODVERSIONS needs to be enabled, see here:
https://source.android.com/devices/architecture/kernel/modular-kernels
FWIW, that doesn't mean that modules are actually used. Devices don't
necessarily have to be using modules.

> Isn't the need for kernel headers for user-space tools something different,
> as this is limited to the uapi versions, which are less (almost not) subject
> to change, compared to the kernel headers needed for compiling kernel
> modules?

Sorry, I should've been clearer. I'm including eBPF/BCC into the
"user-space tools" here. That was in fact my prime motivation in
encouraging Joel at the last LPC to look at this. I've been integrating
the teaching of eBPF into my AOSP debugging and performance analysis
class (see CC courseware here:
http://www.opersys.com/training/android-debug-and-performance), and it
was pretty messy trying to show people how to benefit from such tools
under Android. Joel's present set of patches would obviate this problem.

HTH,

--
Karim Yaghmour
CEO - Opersys inc. / http://www.opersys.com
http://twitter.com/karimyaghmour

2019-03-11 08:04:13

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Karim,

On Sat, Mar 9, 2019 at 10:44 PM Karim Yaghmour
<[email protected]> wrote:
> On 3/9/19 2:26 PM, Geert Uytterhoeven wrote:
> > So how does this work, with kernel images and kernel modules supplied
> > by separate parties, not "bound" by the same kernel headers/API, as they
> > can be replaced separately?
>
> The thing with Android is that there isn't a "one size fits all". Google
> provides guidance, policies and at least one example implementation
> through the Pixel lines.
>
> Each vendor however is allowed a great degree of latitude with regards
> to what they decide to do. Personally, if I was advising a team working
> on an Android device where Joel's patch was available as part of their
> kernel I would just recommend that they build it in -- i.e. not as a
> module. Hence, there would be no module chasing game.

OK, that makes sense, if kernel and modules are separate.
So kernel size increase due to including the headers does matter.

> With regards to Google's guidelines for manufacturers, though, Google
> states that CONFIG_MODVERSIONS needs to be enabled, see here:
> https://source.android.com/devices/architecture/kernel/modular-kernels
> FWIW, that doesn't mean that modules are actually used. Devices don't
> necessarily have to be using modules.

Personally, I had mixed experiences with CONFIG_MODVERSIONS.
But that was a looong time ago, before Android.
Things may have improved.

> > Isn't the need for kernel headers for user-space tools something different,
> > as this is limited to the uapi versions, which are less (almost not) subject
> > to change, compared to the kernel headers needed for compiling kernel
> > modules?
>
> Sorry, I should've been clearer. I'm including eBPF/BCC into the
> "user-space tools" here. That was in fact my prime motivation in
> encouraging Joel at the last LPC to look at this. I've been integrating
> the teaching of eBPF into my AOSP debugging and performance analysis
> class (see CC courseware here:
> http://www.opersys.com/training/android-debug-and-performance), and it
> was pretty messy trying to show people how to benefit from such tools
> under Android. Joel's present set of patches would obviate this problem.

OK.

Now about the actual solution: what is your opinion on embedding e.g.
a squashfs image in the kernel instead, which would be a more generic
solution, not adding more ABI to /proc?

Thanks!

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-03-11 23:38:25

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Sat, 9 Mar 2019 16:44:31 -0500
Karim Yaghmour <[email protected]> wrote:

> Sorry, I should've been clearer. I'm including eBPF/BCC into the
> "user-space tools" here. That was in fact my prime motivation in
> encouraging Joel at the last LPC to look at this. I've been integrating
> the teaching of eBPF into my AOSP debugging and performance analysis
> class (see CC courseware here:
> http://www.opersys.com/training/android-debug-and-performance), and it
> was pretty messy trying to show people how to benefit from such tools
> under Android. Joel's present set of patches would obviate this problem.

I've been reading this thread and staying out for the most part. But I
was thinking about how I could use kernel headers for things like
kprobes, and I want to mention the pony that I would like to have :-)

Are headers really needed, and more importantly, are they enough? What
if userspace is 32bit and the kernel is 64bit. Can ebpf scripts handle
that?

What I would love, is a table that has all structures and their fields
with information about their types, size and signed types. Like the
format fields in the events directory. This way ebpf (and kprobes,
internally in the kernel) could access this table and be able to know
what the data structures of the kernel is).

If you need it for modules, it shouldn't be too hard to take this
information and turn it into headers that building modules could use.

Again, this is my pony, but I'd like to think outside the box here, and
not be so focused on "headers" and see if there's another solution,
that may be even more powerful.

-- Steve

2019-03-11 23:59:21

by Daniel Colascione

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 11, 2019 at 4:37 PM Steven Rostedt <[email protected]> wrote:
>
> On Sat, 9 Mar 2019 16:44:31 -0500
> Karim Yaghmour <[email protected]> wrote:
>
>
> > Sorry, I should've been clearer. I'm including eBPF/BCC into the
> > "user-space tools" here. That was in fact my prime motivation in
> > encouraging Joel at the last LPC to look at this. I've been integrating
> > the teaching of eBPF into my AOSP debugging and performance analysis
> > class (see CC courseware here:
> > http://www.opersys.com/training/android-debug-and-performance), and it
> > was pretty messy trying to show people how to benefit from such tools
> > under Android. Joel's present set of patches would obviate this problem.
>
> I've been reading this thread and staying out for the most part. But I
> was thinking about how I could use kernel headers for things like
> kprobes, and I want to mention the pony that I would like to have :-)
>
> Are headers really needed, and more importantly, are they enough? What
> if userspace is 32bit and the kernel is 64bit. Can ebpf scripts handle
> that?

Why would eBPF care about the bit width of userspace? I've thought a
lot about inspecting user state from the kernel and have some weird
ideas in that area (e.g., why not register eBPF programs to unwind
user stacks?) --- but I think that Joel's work is independent of all
that. I'm curious what you think we can do about userspace. I wish we
could just compile the world with frame pointers, but we can't.

> What I would love, is a table that has all structures and their fields
> with information about their types, size and signed types. Like the
> format fields in the events directory. This way ebpf (and kprobes,
> internally in the kernel) could access this table and be able to know
> what the data structures of the kernel is).

Think of the headers as encoding this information and more and the C
compiler as a magical decoder ring. :-) I totally get the desire for a
metadata format a little less messy than C code, but as a practical
matter, a rich C-compilation pipeline already exists, and the
debuginfo you're proposing doesn't, except in the form of DWARF, which
I think would be even more controversial to embed in the kernel memory
image than headers are. A header blob provides a strict superset of
the information your scheme provides.

2019-03-12 00:41:08

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 11, 2019 at 04:58:28PM -0700, Daniel Colascione wrote:
> On Mon, Mar 11, 2019 at 4:37 PM Steven Rostedt <[email protected]> wrote:
> >
> > On Sat, 9 Mar 2019 16:44:31 -0500
> > Karim Yaghmour <[email protected]> wrote:
> >
> >
> > > Sorry, I should've been clearer. I'm including eBPF/BCC into the
> > > "user-space tools" here. That was in fact my prime motivation in
> > > encouraging Joel at the last LPC to look at this. I've been integrating
> > > the teaching of eBPF into my AOSP debugging and performance analysis
> > > class (see CC courseware here:
> > > http://www.opersys.com/training/android-debug-and-performance), and it
> > > was pretty messy trying to show people how to benefit from such tools
> > > under Android. Joel's present set of patches would obviate this problem.
> >
> > I've been reading this thread and staying out for the most part. But I
> > was thinking about how I could use kernel headers for things like
> > kprobes, and I want to mention the pony that I would like to have :-)
> >
> > Are headers really needed, and more importantly, are they enough? What
> > if userspace is 32bit and the kernel is 64bit. Can ebpf scripts handle
> > that?
>
> Why would eBPF care about the bit width of userspace? I've thought a
> lot about inspecting user state from the kernel and have some weird
> ideas in that area (e.g., why not register eBPF programs to unwind
> user stacks?) --- but I think that Joel's work is independent of all
> that. I'm curious what you think we can do about userspace. I wish we
> could just compile the world with frame pointers, but we can't.

Yeah, most of the usecases for eBPF tracing are all instrumenting the kernel
using kprobes. There are some usecases that instrument userspace with
uprobes but those don't need headers.

> > What I would love, is a table that has all structures and their fields
> > with information about their types, size and signed types. Like the
> > format fields in the events directory. This way ebpf (and kprobes,
> > internally in the kernel) could access this table and be able to know
> > what the data structures of the kernel is).
>
> Think of the headers as encoding this information and more and the C
> compiler as a magical decoder ring. :-) I totally get the desire for a
> metadata format a little less messy than C code, but as a practical
> matter, a rich C-compilation pipeline already exists, and the
> debuginfo you're proposing doesn't, except in the form of DWARF, which
> I think would be even more controversial to embed in the kernel memory
> image than headers are. A header blob provides a strict superset of
> the information your scheme provides.

I think even though the kernel-headers can't have information about all data
structures, they do already contain a lot of data structure definitions we
need already. And anything needed can/should arguably be moved to include/ if
they are really needed for kernel extension by something "external" to the
kernel such as kernel modules or eBPF, right?

In any case, such a solution such as what Steve suggested, still cannot do
what we can with headers - such as build kernel modules on the fly using the
C-compiler without any auto-generation of C code from any debug artifiacts.
Think systemtap working with the module-backend without any need for
linux-headers package on the file system. So such a solution would still be a
bit orthogonal in scope to what this proposed solution can solve IMO.

thanks,

- Joel

2019-03-12 01:23:12

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, 11 Mar 2019 16:58:28 -0700
Daniel Colascione <[email protected]> wrote:

> On Mon, Mar 11, 2019 at 4:37 PM Steven Rostedt <[email protected]> wrote:
> >
> > On Sat, 9 Mar 2019 16:44:31 -0500
> > Karim Yaghmour <[email protected]> wrote:
> >
> >
> > > Sorry, I should've been clearer. I'm including eBPF/BCC into the
> > > "user-space tools" here. That was in fact my prime motivation in
> > > encouraging Joel at the last LPC to look at this. I've been integrating
> > > the teaching of eBPF into my AOSP debugging and performance analysis
> > > class (see CC courseware here:
> > > http://www.opersys.com/training/android-debug-and-performance), and it
> > > was pretty messy trying to show people how to benefit from such tools
> > > under Android. Joel's present set of patches would obviate this problem.
> >
> > I've been reading this thread and staying out for the most part. But I
> > was thinking about how I could use kernel headers for things like
> > kprobes, and I want to mention the pony that I would like to have :-)
> >
> > Are headers really needed, and more importantly, are they enough? What
> > if userspace is 32bit and the kernel is 64bit. Can ebpf scripts handle
> > that?
>
> Why would eBPF care about the bit width of userspace? I've thought a

How do you know the difference between long if you are running on a
64bit kernel in 32bit userspace? I haven't compiled into ebpf byte
code, so I'm clueless here, but I know other tools (PowerTop) got burnt
when it assumed "long" was 4 bytes when digging into the kernel via
trace events, when the kernel was 64bit and user was 32bit.

> lot about inspecting user state from the kernel and have some weird
> ideas in that area (e.g., why not register eBPF programs to unwind
> user stacks?) --- but I think that Joel's work is independent of all
> that. I'm curious what you think we can do about userspace. I wish we
> could just compile the world with frame pointers, but we can't.

I don't care about user space here. I'm talking about the tools that
need to dig into the kernel, but the tools are 32bit and the kernel is
64bit. Of course, if the tools know the kernel is 64bit regardless,
then it doesn't matter.

>
> > What I would love, is a table that has all structures and their
> > fields with information about their types, size and signed types.
> > Like the format fields in the events directory. This way ebpf (and
> > kprobes, internally in the kernel) could access this table and be
> > able to know what the data structures of the kernel is).
>
> Think of the headers as encoding this information and more and the C
> compiler as a magical decoder ring. :-) I totally get the desire for a
> metadata format a little less messy than C code, but as a practical
> matter, a rich C-compilation pipeline already exists, and the
> debuginfo you're proposing doesn't, except in the form of DWARF, which
> I think would be even more controversial to embed in the kernel memory
> image than headers are. A header blob provides a strict superset of
> the information your scheme provides.

We already embed dwarf like information in the kernel. It's called ORC,
and is used by bpf (as well as perf, ftrace and stack dumps) when we
need to read functions. A very small subset of dwarf like data would be
used. Just the data structures, nothing else.

-- Steve

2019-03-12 01:29:06

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, 11 Mar 2019 20:39:12 -0400
Joel Fernandes <[email protected]> wrote:

> I think even though the kernel-headers can't have information about all data
> structures, they do already contain a lot of data structure definitions we
> need already. And anything needed can/should arguably be moved to include/ if
> they are really needed for kernel extension by something "external" to the
> kernel such as kernel modules or eBPF, right?

That's not my worry. I would like to be able to easily walk data
structures from within the kernel, without having to do a lot of work
in userspace to get that information. The kprobe_events could then be
passed type casts or such to access data fields of arguments to
functions and such.

>
> In any case, such a solution such as what Steve suggested, still cannot do
> what we can with headers - such as build kernel modules on the fly using the
> C-compiler without any auto-generation of C code from any debug artifiacts.
> Think systemtap working with the module-backend without any need for
> linux-headers package on the file system. So such a solution would still be a
> bit orthogonal in scope to what this proposed solution can solve IMO.
>

With the information I would like to have, it would be trivial to read
the data to create the header files needed for modules.

-- Steve

2019-03-12 01:39:39

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 11, 2019 at 6:28 PM Steven Rostedt <[email protected]> wrote:
>
> On Mon, 11 Mar 2019 20:39:12 -0400
> Joel Fernandes <[email protected]> wrote:
>
> > I think even though the kernel-headers can't have information about all data
> > structures, they do already contain a lot of data structure definitions we
> > need already. And anything needed can/should arguably be moved to include/ if
> > they are really needed for kernel extension by something "external" to the
> > kernel such as kernel modules or eBPF, right?
>
> That's not my worry. I would like to be able to easily walk data
> structures from within the kernel, without having to do a lot of work
> in userspace to get that information. The kprobe_events could then be
> passed type casts or such to access data fields of arguments to
> functions and such.

Ok.

> > In any case, such a solution such as what Steve suggested, still cannot do
> > what we can with headers - such as build kernel modules on the fly using the
> > C-compiler without any auto-generation of C code from any debug artifiacts.
> > Think systemtap working with the module-backend without any need for
> > linux-headers package on the file system. So such a solution would still be a
> > bit orthogonal in scope to what this proposed solution can solve IMO.
> >
>
> With the information I would like to have, it would be trivial to read
> the data to create the header files needed for modules.

But there are macros and other #define things too. We lose all of them
and can't recreate them from just DWARF (AFAIK). Including
include/generated/autoconf.h which #defines the CONFIG options. For
that we either need headers, or full kernel's sources with build
artifacts.

I do see a use case for the debug info you are talking about as you
mentioned for the kprobe_events argument list types, and I already
thought about it. But it does not seem to work for all the use cases I
am referring to here.

thanks,

- Joel

2019-03-12 01:46:40

by Alexei Starovoitov

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 11, 2019 at 09:28:23PM -0400, Steven Rostedt wrote:
> On Mon, 11 Mar 2019 20:39:12 -0400
> Joel Fernandes <[email protected]> wrote:
>
> > I think even though the kernel-headers can't have information about all data
> > structures, they do already contain a lot of data structure definitions we
> > need already. And anything needed can/should arguably be moved to include/ if
> > they are really needed for kernel extension by something "external" to the
> > kernel such as kernel modules or eBPF, right?
>
> That's not my worry. I would like to be able to easily walk data
> structures from within the kernel, without having to do a lot of work
> in userspace to get that information. The kprobe_events could then be
> passed type casts or such to access data fields of arguments to
> functions and such.
>
> >
> > In any case, such a solution such as what Steve suggested, still cannot do
> > what we can with headers - such as build kernel modules on the fly using the
> > C-compiler without any auto-generation of C code from any debug artifiacts.
> > Think systemtap working with the module-backend without any need for
> > linux-headers package on the file system. So such a solution would still be a
> > bit orthogonal in scope to what this proposed solution can solve IMO.
> >
>
> With the information I would like to have, it would be trivial to read
> the data to create the header files needed for modules.

what you're asking for we already have. It's called BTF.
pahole takes vmlinux dwarf and convert it into ~1Mbyte of BTF that includes
all kernel types.
With gzip it can be compressed further if necessary.
We also have a prototype to generate all_vmlinux_types.h from BTF.
But it's not a substitute for kernel headers.
We've had a long discussion during last LPC regarding this:
http://vger.kernel.org/lpc-bpf2018.html#session-2
tldr: many tracing use cases will be solved with BTF, but kernel headers
are here to stay.

2019-03-12 15:16:34

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Geert,

On 3/11/19 4:03 AM, Geert Uytterhoeven wrote:
[ snip ]
> OK.
>
> Now about the actual solution: what is your opinion on embedding e.g.
> a squashfs image in the kernel instead, which would be a more generic
> solution, not adding more ABI to /proc?

I'm not familiar enough with the intricacies of squashfs to have an
educated opinion, but I hear that it's got its quirks (need for
user-space tools, etc.) and possibly security issues. Also, I wonder
whether it's a generalized solution that still kicks the ABI can down
the road -- ultimately the kernel would still have a path/format/foo for
making kheaders available in that squashfs image and that convention
would become ABI. The only "benefit" being that said ABI wouldn't appear
under /proc, and, tbh, I'm not sure that that's actually a benefit or is
even idiomatic since kconfig.gz is already under /proc. To an extent,
the precedent set by kconfig favors kheaders to also be available in the
same location using a similar mechanism ... i.e. bonus points for
consistency.

But that's my hand-wavy gut-reaction response to your question. I'm sure
others on this thread have far more informed opinions about the
specifics than I could have. My priority was to clarify the basis for
the need being addressed.

Cheers,

--
Karim Yaghmour
CEO - Opersys inc. / http://www.opersys.com
http://twitter.com/karimyaghmour

2019-03-12 15:27:38

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, 11 Mar 2019 18:45:24 -0700
Alexei Starovoitov <[email protected]> wrote:

> what you're asking for we already have. It's called BTF.

Cool.

> pahole takes vmlinux dwarf and convert it into ~1Mbyte of BTF that includes
> all kernel types.
> With gzip it can be compressed further if necessary.
> We also have a prototype to generate all_vmlinux_types.h from BTF.
> But it's not a substitute for kernel headers.
> We've had a long discussion during last LPC regarding this:
> http://vger.kernel.org/lpc-bpf2018.html#session-2
> tldr: many tracing use cases will be solved with BTF, but kernel headers
> are here to stay.

Is this work a result of that session? (I was only able to attend part
of the BPF sessions due to conflicts). I guess I should watch the
videos.

-- Steve

2019-03-13 01:20:52

by Masami Hiramatsu

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, 11 Mar 2019 18:38:49 -0700
Joel Fernandes <[email protected]> wrote:

> On Mon, Mar 11, 2019 at 6:28 PM Steven Rostedt <[email protected]> wrote:
> >
> > On Mon, 11 Mar 2019 20:39:12 -0400
> > Joel Fernandes <[email protected]> wrote:
> >
> > > I think even though the kernel-headers can't have information about all data
> > > structures, they do already contain a lot of data structure definitions we
> > > need already. And anything needed can/should arguably be moved to include/ if
> > > they are really needed for kernel extension by something "external" to the
> > > kernel such as kernel modules or eBPF, right?
> >
> > That's not my worry. I would like to be able to easily walk data
> > structures from within the kernel, without having to do a lot of work
> > in userspace to get that information. The kprobe_events could then be
> > passed type casts or such to access data fields of arguments to
> > functions and such.
>
> Ok.
>
> > > In any case, such a solution such as what Steve suggested, still cannot do
> > > what we can with headers - such as build kernel modules on the fly using the
> > > C-compiler without any auto-generation of C code from any debug artifiacts.
> > > Think systemtap working with the module-backend without any need for
> > > linux-headers package on the file system. So such a solution would still be a
> > > bit orthogonal in scope to what this proposed solution can solve IMO.
> > >
> >
> > With the information I would like to have, it would be trivial to read
> > the data to create the header files needed for modules.
>
> But there are macros and other #define things too. We lose all of them
> and can't recreate them from just DWARF (AFAIK). Including
> include/generated/autoconf.h which #defines the CONFIG options. For
> that we either need headers, or full kernel's sources with build
> artifacts.

What kind of macros would you concern?

> I do see a use case for the debug info you are talking about as you
> mentioned for the kprobe_events argument list types, and I already
> thought about it. But it does not seem to work for all the use cases I
> am referring to here.

But the eBPF is based on kprobe-events. What kind of usage would you
expected? (with macros??)

Thank you,

--
Masami Hiramatsu <[email protected]>

2019-03-14 12:29:13

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

Hi Masami,
Sorry for late reply as I was traveling.

On Wed, Mar 13, 2019 at 10:18:43AM +0900, Masami Hiramatsu wrote:
> On Mon, 11 Mar 2019 18:38:49 -0700
> Joel Fernandes <[email protected]> wrote:
>
> > On Mon, Mar 11, 2019 at 6:28 PM Steven Rostedt <[email protected]> wrote:
> > >
> > > On Mon, 11 Mar 2019 20:39:12 -0400
> > > Joel Fernandes <[email protected]> wrote:
> > >
> > > > I think even though the kernel-headers can't have information about all data
> > > > structures, they do already contain a lot of data structure definitions we
> > > > need already. And anything needed can/should arguably be moved to include/ if
> > > > they are really needed for kernel extension by something "external" to the
> > > > kernel such as kernel modules or eBPF, right?
> > >
> > > That's not my worry. I would like to be able to easily walk data
> > > structures from within the kernel, without having to do a lot of work
> > > in userspace to get that information. The kprobe_events could then be
> > > passed type casts or such to access data fields of arguments to
> > > functions and such.
> >
> > Ok.
> >
> > > > In any case, such a solution such as what Steve suggested, still cannot do
> > > > what we can with headers - such as build kernel modules on the fly using the
> > > > C-compiler without any auto-generation of C code from any debug artifiacts.
> > > > Think systemtap working with the module-backend without any need for
> > > > linux-headers package on the file system. So such a solution would still be a
> > > > bit orthogonal in scope to what this proposed solution can solve IMO.
> > > >
> > >
> > > With the information I would like to have, it would be trivial to read
> > > the data to create the header files needed for modules.
> >
> > But there are macros and other #define things too. We lose all of them
> > and can't recreate them from just DWARF (AFAIK). Including
> > include/generated/autoconf.h which #defines the CONFIG options. For
> > that we either need headers, or full kernel's sources with build
> > artifacts.
>
> What kind of macros would you concern?
>
> > I do see a use case for the debug info you are talking about as you
> > mentioned for the kprobe_events argument list types, and I already
> > thought about it. But it does not seem to work for all the use cases I
> > am referring to here.
>
> But the eBPF is based on kprobe-events. What kind of usage would you
> expected? (with macros??)

eBPF C programs are compiled with kernel headers. They can execute inline
functions or refer to macros in the kernel headers. They are similar to
kernel modules where you build a C program that then later is executed in
kernel context. It goes through the whole compiler pipeline. This is slightly
different usage from pure kprobe-events. Also eBPF kprobe programs need
LINUX_VERSION_CODE (or similarly named) macro which it provides to the bpf(2)
syscall when loading kprobe programs. This is because eBPF implementation in
the kernel checks if the eBPF programs that use kprobes are being loaded
against the right kernel.

thanks,

- Joel

2019-03-15 13:15:09

by Masami Hiramatsu

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Thu, 14 Mar 2019 08:27:48 -0400
Joel Fernandes <[email protected]> wrote:

> > But the eBPF is based on kprobe-events. What kind of usage would you
> > expected? (with macros??)
>
> eBPF C programs are compiled with kernel headers. They can execute inline
> functions or refer to macros in the kernel headers. They are similar to
> kernel modules where you build a C program that then later is executed in
> kernel context. It goes through the whole compiler pipeline. This is slightly
> different usage from pure kprobe-events. Also eBPF kprobe programs need
> LINUX_VERSION_CODE (or similarly named) macro which it provides to the bpf(2)
> syscall when loading kprobe programs. This is because eBPF implementation in
> the kernel checks if the eBPF programs that use kprobes are being loaded
> against the right kernel.

Ah, I got it. It's similar to SystemTap. :)

Thank you,

--
Masami Hiramatsu <[email protected]>

2019-03-18 19:12:45

by Daniel Colascione

[permalink] [raw]

Subject: Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel

On Mon, Mar 18, 2019 at 11:58 AM Alan Cox <[email protected]> wrote:
>
> > I think the compressed tarball is much simpler/easier overall. If
> > someone really wants the filesystem, they just uncompress it into a
> > tmpfs mount. It's much less moving kernel code to worry about.
>
> There is an even simpler approach. The people who want this for whatever
> strange reason are Android folks. Android lives on flash, so all they have
> to do is put the headers in a flash file system that is updated with the
> kernel and mount it wherever they like. Simple matter of a bit of
> devicetree no ?

Did you read the rest of the thread? We talked a lot about the
problems with filesystem-based approaches. It's not just "Android
folks" who want BPF-based tools to Just Work, and it's not "strange"
to want that --- what's strange is this reflexive opposition to a
pragmatic feature (one that nobody has to use) that will make a lot of
system analysis cases Just Work. There's nothing wrong with having the
kernel describe itself, and there's no reason to push tons of
error-prone metadata tracking to userspace when the kernel can just
talk about itself in a guaranteed-correct manner. Every argument
against this work is also an argument against /proc/config.gz.
Completely unsurprisingly, /proc/config.gz is in practice super
useful.

2019-03-18 21:13:03