linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
@ 2019-03-01 16:08 Joel Fernandes (Google)
  2019-03-01 16:08 ` [PATCH v4 2/2] Add selftests for module build using in-kernel headers Joel Fernandes (Google)
                   ` (5 more replies)
  0 siblings, 6 replies; 48+ messages in thread
From: Joel Fernandes (Google) @ 2019-03-01 16:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google),
	Andrew Morton, ast, atishp04, dancol, Dan Williams,
	dietmar.eggemann, gregkh, Guenter Roeck, Jonathan Corbet,
	karim.yaghmour, Kees Cook, kernel-team, linux-doc,
	linux-kselftest, linux-trace-devel, Manoj Rao, Masahiro Yamada,
	mhiramat, qais.yousef, rdunlap, rostedt, Shuah Khan, yhs

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) <joel@joelfernandes.org>
---

    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

^ permalink raw reply related	[flat|nested] 48+ messages in thread

* [PATCH v4 2/2] Add selftests for module build using in-kernel headers
  2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
@ 2019-03-01 16:08 ` Joel Fernandes (Google)
  2019-03-02 21:59 ` [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel kbuild test robot
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 48+ messages in thread
From: Joel Fernandes (Google) @ 2019-03-01 16:08 UTC (permalink / raw)
  To: linux-kernel
  Cc: Joel Fernandes (Google),
	Andrew Morton, ast, atishp04, dancol, Dan Williams,
	dietmar.eggemann, gregkh, Guenter Roeck, Jonathan Corbet,
	karim.yaghmour, Kees Cook, kernel-team, linux-doc,
	linux-kselftest, linux-trace-devel, Manoj Rao, Masahiro Yamada,
	mhiramat, qais.yousef, rdunlap, rostedt, Shuah Khan, yhs

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) <joel@joelfernandes.org>
---
 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


^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
  2019-03-01 16:08 ` [PATCH v4 2/2] Add selftests for module build using in-kernel headers Joel Fernandes (Google)
@ 2019-03-02 21:59 ` kbuild test robot
  2019-03-03 16:11   ` Joel Fernandes
  2019-03-03  2:04 ` kbuild test robot
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 48+ messages in thread
From: kbuild test robot @ 2019-03-02 21:59 UTC (permalink / raw)
  To: Joel Fernandes (Google)
  Cc: kbuild-all, linux-kernel, Joel Fernandes (Google),
	Andrew Morton, ast, atishp04, dancol, Dan Williams,
	dietmar.eggemann, gregkh, Guenter Roeck, Jonathan Corbet,
	karim.yaghmour, Kees Cook, kernel-team, linux-doc,
	linux-kselftest, linux-trace-devel, Manoj Rao, Masahiro Yamada,
	mhiramat, qais.yousef, rdunlap, rostedt, Shuah Khan, yhs

[-- Attachment #1: Type: text/plain, Size: 1103 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 50810 bytes --]

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
  2019-03-01 16:08 ` [PATCH v4 2/2] Add selftests for module build using in-kernel headers Joel Fernandes (Google)
  2019-03-02 21:59 ` [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel kbuild test robot
@ 2019-03-03  2:04 ` kbuild test robot
  2019-03-04 14:00 ` Qais Yousef
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 48+ messages in thread
From: kbuild test robot @ 2019-03-03  2:04 UTC (permalink / raw)
  To: Joel Fernandes (Google)
  Cc: kbuild-all, linux-kernel, Joel Fernandes (Google),
	Andrew Morton, ast, atishp04, dancol, Dan Williams,
	dietmar.eggemann, gregkh, Guenter Roeck, Jonathan Corbet,
	karim.yaghmour, Kees Cook, kernel-team, linux-doc,
	linux-kselftest, linux-trace-devel, Manoj Rao, Masahiro Yamada,
	mhiramat, qais.yousef, rdunlap, rostedt, Shuah Khan, yhs

[-- Attachment #1: Type: text/plain, Size: 1126 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 56201 bytes --]

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-02 21:59 ` [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel kbuild test robot
@ 2019-03-03 16:11   ` Joel Fernandes
  2019-03-06 12:26     ` Masahiro Yamada
  2019-03-06 18:16     ` Joel Fernandes
  0 siblings, 2 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-03 16:11 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Joel Fernandes (Google),
	kbuild-all, LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Greg Kroah-Hartman, Guenter Roeck, Jonathan Corbet,
	Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, linux-kselftest, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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 <lkp@intel.com> 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 kernel-team+unsubscribe@android.com.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
                   ` (2 preceding siblings ...)
  2019-03-03  2:04 ` kbuild test robot
@ 2019-03-04 14:00 ` Qais Yousef
  2019-03-05 16:27   ` Joel Fernandes
  2019-03-04 22:48 ` Dietmar Eggemann
  2019-03-07  8:58 ` Geert Uytterhoeven
  5 siblings, 1 reply; 48+ messages in thread
From: Qais Yousef @ 2019-03-04 14:00 UTC (permalink / raw)
  To: Joel Fernandes (Google)
  Cc: linux-kernel, Andrew Morton, ast, atishp04, dancol, Dan Williams,
	dietmar.eggemann, gregkh, Guenter Roeck, Jonathan Corbet,
	karim.yaghmour, Kees Cook, kernel-team, linux-doc,
	linux-kselftest, linux-trace-devel, Manoj Rao, Masahiro Yamada,
	mhiramat, rdunlap, rostedt, Shuah Khan, yhs

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) <joel@joelfernandes.org>

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 <qais.yousef@arm.com>

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
                   ` (3 preceding siblings ...)
  2019-03-04 14:00 ` Qais Yousef
@ 2019-03-04 22:48 ` Dietmar Eggemann
  2019-03-05 16:25   ` Joel Fernandes
  2019-03-07  8:58 ` Geert Uytterhoeven
  5 siblings, 1 reply; 48+ messages in thread
From: Dietmar Eggemann @ 2019-03-04 22:48 UTC (permalink / raw)
  To: Joel Fernandes (Google), linux-kernel
  Cc: Andrew Morton, ast, atishp04, dancol, Dan Williams, gregkh,
	Guenter Roeck, Jonathan Corbet, karim.yaghmour, Kees Cook,
	kernel-team, linux-doc, linux-kselftest, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, mhiramat, qais.yousef, rdunlap,
	rostedt, Shuah Khan, yhs

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) <joel@joelfernandes.org>
> ---
> 
>      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 <dietmar.eggemann@arm.com>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-04 22:48 ` Dietmar Eggemann
@ 2019-03-05 16:25   ` Joel Fernandes
  0 siblings, 0 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-05 16:25 UTC (permalink / raw)
  To: Dietmar Eggemann
  Cc: linux-kernel, Andrew Morton, ast, atishp04, dancol, Dan Williams,
	gregkh, Guenter Roeck, Jonathan Corbet, karim.yaghmour,
	Kees Cook, kernel-team, linux-doc, linux-kselftest,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, mhiramat,
	qais.yousef, rdunlap, rostedt, Shuah Khan, yhs

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) <joel@joelfernandes.org>
> > ---
> > 
> >      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 <dietmar.eggemann@arm.com>

Thank you! Will repost with the tags soon.

- Joel


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-04 14:00 ` Qais Yousef
@ 2019-03-05 16:27   ` Joel Fernandes
  0 siblings, 0 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-05 16:27 UTC (permalink / raw)
  To: Qais Yousef
  Cc: linux-kernel, Andrew Morton, ast, atishp04, dancol, Dan Williams,
	dietmar.eggemann, gregkh, Guenter Roeck, Jonathan Corbet,
	karim.yaghmour, Kees Cook, kernel-team, linux-doc,
	linux-kselftest, linux-trace-devel, Manoj Rao, Masahiro Yamada,
	mhiramat, rdunlap, rostedt, Shuah Khan, yhs

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) <joel@joelfernandes.org>
> 
> 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 <qais.yousef@arm.com>
> 
> 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


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-03 16:11   ` Joel Fernandes
@ 2019-03-06 12:26     ` Masahiro Yamada
  2019-03-06 17:49       ` Joel Fernandes
  2019-03-07 23:23       ` Justin Capella
  2019-03-06 18:16     ` Joel Fernandes
  1 sibling, 2 replies; 48+ messages in thread
From: Masahiro Yamada @ 2019-03-06 12:26 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: kbuild test robot, Joel Fernandes (Google),
	kbuild-all, LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Greg Kroah-Hartman, Guenter Roeck, Jonathan Corbet,
	Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

On Mon, Mar 4, 2019 at 1:15 AM Joel Fernandes <joelaf@google.com> 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 <lkp@intel.com> 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 kernel-team+unsubscribe@android.com.



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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-06 12:26     ` Masahiro Yamada
@ 2019-03-06 17:49       ` Joel Fernandes
  2019-03-07  4:59         ` Masahiro Yamada
  2019-03-07 23:23       ` Justin Capella
  1 sibling, 1 reply; 48+ messages in thread
From: Joel Fernandes @ 2019-03-06 17:49 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: kbuild test robot, kbuild-all, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Daniel Colascione, Dan Williams,
	Dietmar Eggemann, Greg Kroah-Hartman, Guenter Roeck,
	Jonathan Corbet, Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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 <joelaf@google.com> 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 <lkp@intel.com> 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 kernel-team+unsubscribe@android.com.
> 
> 
> 
> 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


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-03 16:11   ` Joel Fernandes
  2019-03-06 12:26     ` Masahiro Yamada
@ 2019-03-06 18:16     ` Joel Fernandes
  2019-03-07  4:54       ` Masahiro Yamada
  1 sibling, 1 reply; 48+ messages in thread
From: Joel Fernandes @ 2019-03-06 18:16 UTC (permalink / raw)
  To: kbuild test robot
  Cc: kbuild-all, LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Greg Kroah-Hartman, Guenter Roeck, Jonathan Corbet,
	Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, linux-kselftest, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

 
> On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <lkp@intel.com> 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


^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-06 18:16     ` Joel Fernandes
@ 2019-03-07  4:54       ` Masahiro Yamada
  0 siblings, 0 replies; 48+ messages in thread
From: Masahiro Yamada @ 2019-03-07  4:54 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: kbuild test robot, kbuild-all, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Daniel Colascione, Dan Williams,
	Dietmar Eggemann, Greg Kroah-Hartman, Guenter Roeck,
	Jonathan Corbet, Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

On Thu, Mar 7, 2019 at 5:31 AM Joel Fernandes <joel@joelfernandes.org> wrote:
>
>
> > On Sat, Mar 2, 2019 at 2:00 PM kbuild test robot <lkp@intel.com> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-06 17:49       ` Joel Fernandes
@ 2019-03-07  4:59         ` Masahiro Yamada
  2019-03-07 14:54           ` Joel Fernandes
  0 siblings, 1 reply; 48+ messages in thread
From: Masahiro Yamada @ 2019-03-07  4:59 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: kbuild test robot, kbuild-all, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Daniel Colascione, Dan Williams,
	Dietmar Eggemann, Greg Kroah-Hartman, Guenter Roeck,
	Jonathan Corbet, Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

On Thu, Mar 7, 2019 at 5:13 AM Joel Fernandes <joel@joelfernandes.org> 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 <joelaf@google.com> 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 <lkp@intel.com> 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 kernel-team+unsubscribe@android.com.
> >
> >
> >
> > 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
>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
                   ` (4 preceding siblings ...)
  2019-03-04 22:48 ` Dietmar Eggemann
@ 2019-03-07  8:58 ` Geert Uytterhoeven
  2019-03-07 15:03   ` Joel Fernandes
  5 siblings, 1 reply; 48+ messages in thread
From: Geert Uytterhoeven @ 2019-03-07  8:58 UTC (permalink / raw)
  To: Joel Fernandes (Google)
  Cc: Linux Kernel Mailing List, Andrew Morton, Alexei Starovoitov,
	atishp04, dancol, Dan Williams, Dietmar Eggemann, Greg KH,
	Guenter Roeck, Jonathan Corbet, karim.yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, qais.yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, yhs

Hi Joel,

On Fri, Mar 1, 2019 at 5:10 PM Joel Fernandes (Google)
<joel@joelfernandes.org> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-07  4:59         ` Masahiro Yamada
@ 2019-03-07 14:54           ` Joel Fernandes
  0 siblings, 0 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-07 14:54 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: kbuild test robot, kbuild-all, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Daniel Colascione, Dan Williams,
	Dietmar Eggemann, Greg Kroah-Hartman, Guenter Roeck,
	Jonathan Corbet, Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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)" <joel@joelfernandes.org>
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: qais.yousef@arm.com
Tested-by: dietmar.eggemann@arm.com
Tested-by: linux@manojrajarao.com
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
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) <joel@joelfernandes.org>
---
 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


^ permalink raw reply related	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-07  8:58 ` Geert Uytterhoeven
@ 2019-03-07 15:03   ` Joel Fernandes
  2019-03-07 15:23     ` Greg KH
  2019-03-08  8:53     ` Geert Uytterhoeven
  0 siblings, 2 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-07 15:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Linux Kernel Mailing List, Andrew Morton, Alexei Starovoitov,
	atishp04, dancol, Dan Williams, Dietmar Eggemann, Greg KH,
	Guenter Roeck, Jonathan Corbet, karim.yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, qais.yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, yhs

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)
> <joel@joelfernandes.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-07 15:03   ` Joel Fernandes
@ 2019-03-07 15:23     ` Greg KH
  2019-03-07 16:54       ` Joel Fernandes
       [not found]       ` <20190318185742.109dee5c@alans-desktop>
  2019-03-08  8:53     ` Geert Uytterhoeven
  1 sibling, 2 replies; 48+ messages in thread
From: Greg KH @ 2019-03-07 15:23 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Geert Uytterhoeven, Linux Kernel Mailing List, Andrew Morton,
	Alexei Starovoitov, atishp04, dancol, Dan Williams,
	Dietmar Eggemann, Guenter Roeck, Jonathan Corbet, karim.yaghmour,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, qais.yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, yhs

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)
> > <joel@joelfernandes.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-07 15:23     ` Greg KH
@ 2019-03-07 16:54       ` Joel Fernandes
       [not found]       ` <20190318185742.109dee5c@alans-desktop>
  1 sibling, 0 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-07 16:54 UTC (permalink / raw)
  To: Greg KH
  Cc: Geert Uytterhoeven, Linux Kernel Mailing List, Andrew Morton,
	Alexei Starovoitov, atishp04, dancol, Dan Williams,
	Dietmar Eggemann, Guenter Roeck, Jonathan Corbet, karim.yaghmour,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, qais.yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, yhs

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)
> > > <joel@joelfernandes.org> 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


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-06 12:26     ` Masahiro Yamada
  2019-03-06 17:49       ` Joel Fernandes
@ 2019-03-07 23:23       ` Justin Capella
  1 sibling, 0 replies; 48+ messages in thread
From: Justin Capella @ 2019-03-07 23:23 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Joel Fernandes, kbuild test robot, Joel Fernandes (Google),
	kbuild-all, LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Greg Kroah-Hartman, Guenter Roeck, Jonathan Corbet,
	Karim Yaghmour, Kees Cook, Cc: Android Kernel,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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
<yamada.masahiro@socionext.com> wrote:
>
> On Mon, Mar 4, 2019 at 1:15 AM Joel Fernandes <joelaf@google.com> 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 <lkp@intel.com> 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 kernel-team+unsubscribe@android.com.
>
>
>
> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-07 15:03   ` Joel Fernandes
  2019-03-07 15:23     ` Greg KH
@ 2019-03-08  8:53     ` Geert Uytterhoeven
  2019-03-08 13:42       ` Joel Fernandes
  1 sibling, 1 reply; 48+ messages in thread
From: Geert Uytterhoeven @ 2019-03-08  8:53 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Linux Kernel Mailing List, Andrew Morton, Alexei Starovoitov,
	atishp04, dancol, Dan Williams, Dietmar Eggemann, Greg KH,
	Guenter Roeck, Jonathan Corbet, karim.yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, qais.yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, yhs

Hi Joel,

On Thu, Mar 7, 2019 at 4:03 PM Joel Fernandes <joel@joelfernandes.org> 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)
> > <joel@joelfernandes.org> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08  8:53     ` Geert Uytterhoeven
@ 2019-03-08 13:42       ` Joel Fernandes
  2019-03-08 13:57         ` Enrico Weigelt, metux IT consult
  2019-03-08 14:02         ` Greg KH
  0 siblings, 2 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-08 13:42 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Daniel Colascione, Dan Williams, Dietmar Eggemann, Greg KH,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

HI Geert,

On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <geert@linux-m68k.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08 13:42       ` Joel Fernandes
@ 2019-03-08 13:57         ` Enrico Weigelt, metux IT consult
  2019-03-08 14:04           ` Greg KH
  2019-03-08 14:02         ` Greg KH
  1 sibling, 1 reply; 48+ messages in thread
From: Enrico Weigelt, metux IT consult @ 2019-03-08 13:57 UTC (permalink / raw)
  To: Joel Fernandes, Geert Uytterhoeven
  Cc: LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Daniel Colascione, Dan Williams, Dietmar Eggemann, Greg KH,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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
info@metux.net -- +49-151-27565287

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08 13:42       ` Joel Fernandes
  2019-03-08 13:57         ` Enrico Weigelt, metux IT consult
@ 2019-03-08 14:02         ` Greg KH
  2019-03-08 17:58           ` Joel Fernandes
  2019-03-08 17:59           ` Geert Uytterhoeven
  1 sibling, 2 replies; 48+ messages in thread
From: Greg KH @ 2019-03-08 14:02 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Geert Uytterhoeven, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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 <geert@linux-m68k.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08 13:57         ` Enrico Weigelt, metux IT consult
@ 2019-03-08 14:04           ` Greg KH
  0 siblings, 0 replies; 48+ messages in thread
From: Greg KH @ 2019-03-08 14:04 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: Joel Fernandes, Geert Uytterhoeven, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Daniel Colascione, Dan Williams,
	Dietmar Eggemann, Guenter Roeck, Jonathan Corbet, Karim Yaghmour,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08 14:02         ` Greg KH
@ 2019-03-08 17:58           ` Joel Fernandes
  2019-03-08 17:59           ` Geert Uytterhoeven
  1 sibling, 0 replies; 48+ messages in thread
From: Joel Fernandes @ 2019-03-08 17:58 UTC (permalink / raw)
  To: Greg KH
  Cc: Geert Uytterhoeven, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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 <geert@linux-m68k.org> 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


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08 14:02         ` Greg KH
  2019-03-08 17:58           ` Joel Fernandes
@ 2019-03-08 17:59           ` Geert Uytterhoeven
  2019-03-09  7:16             ` Greg KH
  1 sibling, 1 reply; 48+ messages in thread
From: Geert Uytterhoeven @ 2019-03-08 17:59 UTC (permalink / raw)
  To: Greg KH
  Cc: Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

Hi Greg,

On Fri, Mar 8, 2019 at 6:05 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <geert@linux-m68k.org> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-08 17:59           ` Geert Uytterhoeven
@ 2019-03-09  7:16             ` Greg KH
  2019-03-09 11:40               ` Geert Uytterhoeven
  0 siblings, 1 reply; 48+ messages in thread
From: Greg KH @ 2019-03-09  7:16 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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 <gregkh@linuxfoundation.org> wrote:
> > On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <geert@linux-m68k.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09  7:16             ` Greg KH
@ 2019-03-09 11:40               ` Geert Uytterhoeven
  2019-03-09 12:11                 ` Greg KH
  0 siblings, 1 reply; 48+ messages in thread
From: Geert Uytterhoeven @ 2019-03-09 11:40 UTC (permalink / raw)
  To: Greg KH
  Cc: Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

Hi Greg,

On Sat, Mar 9, 2019 at 8:16 AM Greg KH <gregkh@linuxfoundation.org> 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 <gregkh@linuxfoundation.org> wrote:
> > > On Fri, Mar 08, 2019 at 05:42:32AM -0800, Joel Fernandes wrote:
> > > > On Fri, Mar 8, 2019, 3:53 AM Geert Uytterhoeven <geert@linux-m68k.org> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09 11:40               ` Geert Uytterhoeven
@ 2019-03-09 12:11                 ` Greg KH
  2019-03-09 16:51                   ` Karim Yaghmour
  0 siblings, 1 reply; 48+ messages in thread
From: Greg KH @ 2019-03-09 12:11 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Karim Yaghmour, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Steven Rostedt, Shuah Khan, Yonghong Song

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09 12:11                 ` Greg KH
@ 2019-03-09 16:51                   ` Karim Yaghmour
  2019-03-09 19:26                     ` Geert Uytterhoeven
  0 siblings, 1 reply; 48+ messages in thread
From: Karim Yaghmour @ 2019-03-09 16:51 UTC (permalink / raw)
  To: Greg KH, Geert Uytterhoeven
  Cc: Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Steven Rostedt, Shuah Khan,
	Yonghong Song


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. / www.opersys.com
http://twitter.com/karimyaghmour

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09 16:51                   ` Karim Yaghmour
@ 2019-03-09 19:26                     ` Geert Uytterhoeven
  2019-03-09 21:44                       ` Karim Yaghmour
  0 siblings, 1 reply; 48+ messages in thread
From: Geert Uytterhoeven @ 2019-03-09 19:26 UTC (permalink / raw)
  To: Karim Yaghmour
  Cc: Greg KH, Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Steven Rostedt, Shuah Khan,
	Yonghong Song

Hi Karim,

Thanks for the explanation!

On Sat, Mar 9, 2019 at 5:52 PM Karim Yaghmour
<karim.yaghmour@opersys.com> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09 19:26                     ` Geert Uytterhoeven
@ 2019-03-09 21:44                       ` Karim Yaghmour
  2019-03-11  8:03                         ` Geert Uytterhoeven
  2019-03-11 23:36                         ` Steven Rostedt
  0 siblings, 2 replies; 48+ messages in thread
From: Karim Yaghmour @ 2019-03-09 21:44 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg KH, Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Steven Rostedt, Shuah Khan,
	Yonghong Song


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. / www.opersys.com
http://twitter.com/karimyaghmour

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09 21:44                       ` Karim Yaghmour
@ 2019-03-11  8:03                         ` Geert Uytterhoeven
  2019-03-12 15:15                           ` Karim Yaghmour
  2019-03-11 23:36                         ` Steven Rostedt
  1 sibling, 1 reply; 48+ messages in thread
From: Geert Uytterhoeven @ 2019-03-11  8:03 UTC (permalink / raw)
  To: Karim Yaghmour
  Cc: Greg KH, Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Steven Rostedt, Shuah Khan,
	Yonghong Song

Hi Karim,

On Sat, Mar 9, 2019 at 10:44 PM Karim Yaghmour
<karim.yaghmour@opersys.com> 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 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-09 21:44                       ` Karim Yaghmour
  2019-03-11  8:03                         ` Geert Uytterhoeven
@ 2019-03-11 23:36                         ` Steven Rostedt
  2019-03-11 23:58                           ` Daniel Colascione
  1 sibling, 1 reply; 48+ messages in thread
From: Steven Rostedt @ 2019-03-11 23:36 UTC (permalink / raw)
  To: Karim Yaghmour
  Cc: Geert Uytterhoeven, Greg KH, Joel Fernandes, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Daniel Colascione, Dan Williams,
	Dietmar Eggemann, Guenter Roeck, Jonathan Corbet, Kees Cook,
	Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

On Sat, 9 Mar 2019 16:44:31 -0500
Karim Yaghmour <karim.yaghmour@opersys.com> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-11 23:36                         ` Steven Rostedt
@ 2019-03-11 23:58                           ` Daniel Colascione
  2019-03-12  0:39                             ` Joel Fernandes
  2019-03-12  1:22                             ` Steven Rostedt
  0 siblings, 2 replies; 48+ messages in thread
From: Daniel Colascione @ 2019-03-11 23:58 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Karim Yaghmour, Geert Uytterhoeven, Greg KH, Joel Fernandes,
	LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Dan Williams, Dietmar Eggemann, Guenter Roeck, Jonathan Corbet,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, Mar 11, 2019 at 4:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Sat, 9 Mar 2019 16:44:31 -0500
> Karim Yaghmour <karim.yaghmour@opersys.com> 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.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-11 23:58                           ` Daniel Colascione
@ 2019-03-12  0:39                             ` Joel Fernandes
  2019-03-12  1:28                               ` Steven Rostedt
  2019-03-12  1:22                             ` Steven Rostedt
  1 sibling, 1 reply; 48+ messages in thread
From: Joel Fernandes @ 2019-03-12  0:39 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Steven Rostedt, Karim Yaghmour, Geert Uytterhoeven, Greg KH,
	LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Dan Williams, Dietmar Eggemann, Guenter Roeck, Jonathan Corbet,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, Mar 11, 2019 at 04:58:28PM -0700, Daniel Colascione wrote:
> On Mon, Mar 11, 2019 at 4:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Sat, 9 Mar 2019 16:44:31 -0500
> > Karim Yaghmour <karim.yaghmour@opersys.com> 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


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-11 23:58                           ` Daniel Colascione
  2019-03-12  0:39                             ` Joel Fernandes
@ 2019-03-12  1:22                             ` Steven Rostedt
  1 sibling, 0 replies; 48+ messages in thread
From: Steven Rostedt @ 2019-03-12  1:22 UTC (permalink / raw)
  To: Daniel Colascione
  Cc: Karim Yaghmour, Geert Uytterhoeven, Greg KH, Joel Fernandes,
	LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Dan Williams, Dietmar Eggemann, Guenter Roeck, Jonathan Corbet,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, 11 Mar 2019 16:58:28 -0700
Daniel Colascione <dancol@google.com> wrote:

> On Mon, Mar 11, 2019 at 4:37 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Sat, 9 Mar 2019 16:44:31 -0500
> > Karim Yaghmour <karim.yaghmour@opersys.com> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-12  0:39                             ` Joel Fernandes
@ 2019-03-12  1:28                               ` Steven Rostedt
  2019-03-12  1:38                                 ` Joel Fernandes
  2019-03-12  1:45                                 ` Alexei Starovoitov
  0 siblings, 2 replies; 48+ messages in thread
From: Steven Rostedt @ 2019-03-12  1:28 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Daniel Colascione, Karim Yaghmour, Geert Uytterhoeven, Greg KH,
	LKML, Andrew Morton, Alexei Starovoitov, atish patra,
	Dan Williams, Dietmar Eggemann, Guenter Roeck, Jonathan Corbet,
	Kees Cook, Android Kernel Team, open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK, linux-trace-devel,
	Manoj Rao, Masahiro Yamada, Masami Hiramatsu, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, 11 Mar 2019 20:39:12 -0400
Joel Fernandes <joel@joelfernandes.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-12  1:28                               ` Steven Rostedt
@ 2019-03-12  1:38                                 ` Joel Fernandes
  2019-03-13  1:18                                   ` Masami Hiramatsu
  2019-03-12  1:45                                 ` Alexei Starovoitov
  1 sibling, 1 reply; 48+ messages in thread
From: Joel Fernandes @ 2019-03-12  1:38 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Joel Fernandes, Daniel Colascione, Karim Yaghmour,
	Geert Uytterhoeven, Greg KH, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, Mar 11, 2019 at 6:28 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Mon, 11 Mar 2019 20:39:12 -0400
> Joel Fernandes <joel@joelfernandes.org> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-12  1:28                               ` Steven Rostedt
  2019-03-12  1:38                                 ` Joel Fernandes
@ 2019-03-12  1:45                                 ` Alexei Starovoitov
  2019-03-12 15:26                                   ` Steven Rostedt
  1 sibling, 1 reply; 48+ messages in thread
From: Alexei Starovoitov @ 2019-03-12  1:45 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Joel Fernandes, Daniel Colascione, Karim Yaghmour,
	Geert Uytterhoeven, Greg KH, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, Mar 11, 2019 at 09:28:23PM -0400, Steven Rostedt wrote:
> On Mon, 11 Mar 2019 20:39:12 -0400
> Joel Fernandes <joel@joelfernandes.org> 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.


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-11  8:03                         ` Geert Uytterhoeven
@ 2019-03-12 15:15                           ` Karim Yaghmour
  0 siblings, 0 replies; 48+ messages in thread
From: Karim Yaghmour @ 2019-03-12 15:15 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Greg KH, Joel Fernandes, LKML, Andrew Morton, Alexei Starovoitov,
	atish patra, Daniel Colascione, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Steven Rostedt, Shuah Khan,
	Yonghong Song


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. / www.opersys.com
http://twitter.com/karimyaghmour

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-12  1:45                                 ` Alexei Starovoitov
@ 2019-03-12 15:26                                   ` Steven Rostedt
  0 siblings, 0 replies; 48+ messages in thread
From: Steven Rostedt @ 2019-03-12 15:26 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Joel Fernandes, Daniel Colascione, Karim Yaghmour,
	Geert Uytterhoeven, Greg KH, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, 11 Mar 2019 18:45:24 -0700
Alexei Starovoitov <alexei.starovoitov@gmail.com> 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

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-12  1:38                                 ` Joel Fernandes
@ 2019-03-13  1:18                                   ` Masami Hiramatsu
  2019-03-14 12:27                                     ` Joel Fernandes
  0 siblings, 1 reply; 48+ messages in thread
From: Masami Hiramatsu @ 2019-03-13  1:18 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Steven Rostedt, Joel Fernandes, Daniel Colascione,
	Karim Yaghmour, Geert Uytterhoeven, Greg KH, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	Qais Yousef, Randy Dunlap, Shuah Khan, Yonghong Song

On Mon, 11 Mar 2019 18:38:49 -0700
Joel Fernandes <joelaf@google.com> wrote:

> On Mon, Mar 11, 2019 at 6:28 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> >
> > On Mon, 11 Mar 2019 20:39:12 -0400
> > Joel Fernandes <joel@joelfernandes.org> 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 <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-13  1:18                                   ` Masami Hiramatsu
@ 2019-03-14 12:27                                     ` Joel Fernandes
  2019-03-15 13:14                                       ` Masami Hiramatsu
  0 siblings, 1 reply; 48+ messages in thread
From: Joel Fernandes @ 2019-03-14 12:27 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Steven Rostedt, Daniel Colascione, Karim Yaghmour,
	Geert Uytterhoeven, Greg KH, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

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 <joelaf@google.com> wrote:
> 
> > On Mon, Mar 11, 2019 at 6:28 PM Steven Rostedt <rostedt@goodmis.org> wrote:
> > >
> > > On Mon, 11 Mar 2019 20:39:12 -0400
> > > Joel Fernandes <joel@joelfernandes.org> 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


^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
  2019-03-14 12:27                                     ` Joel Fernandes
@ 2019-03-15 13:14                                       ` Masami Hiramatsu
  0 siblings, 0 replies; 48+ messages in thread
From: Masami Hiramatsu @ 2019-03-15 13:14 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Steven Rostedt, Daniel Colascione, Karim Yaghmour,
	Geert Uytterhoeven, Greg KH, LKML, Andrew Morton,
	Alexei Starovoitov, atish patra, Dan Williams, Dietmar Eggemann,
	Guenter Roeck, Jonathan Corbet, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION, open list:KERNEL SELFTEST FRAMEWORK,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Qais Yousef,
	Randy Dunlap, Shuah Khan, Yonghong Song

On Thu, 14 Mar 2019 08:27:48 -0400
Joel Fernandes <joel@joelfernandes.org> 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 <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
       [not found]       ` <20190318185742.109dee5c@alans-desktop>
@ 2019-03-18 19:11         ` Daniel Colascione
  2019-03-18 21:11         ` Karim Yaghmour
  1 sibling, 0 replies; 48+ messages in thread
From: Daniel Colascione @ 2019-03-18 19:11 UTC (permalink / raw)
  To: Alan Cox
  Cc: Greg KH, Joel Fernandes, Geert Uytterhoeven,
	Linux Kernel Mailing List, Andrew Morton, Alexei Starovoitov,
	atish patra, Dan Williams, Dietmar Eggemann, Guenter Roeck,
	Jonathan Corbet, Karim Yaghmour, Kees Cook, Android Kernel Team,
	open list:DOCUMENTATION,
	open list:KERNEL SELFTEST FRAMEWORK
	<linux-kselftest@vger.kernel.org>,
	linux-trace-devel@vger.kernel.org,
	Manoj Rao <linux@manojrajarao.com>,
	Masahiro Yamada <yamada.masahiro@socionext.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	qais.yousef@arm.com, Randy Dunlap <rdunlap@infradead.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Shuah Khan <shuah@kernel.org>,

On Mon, Mar 18, 2019 at 11:58 AM Alan Cox <gnomes@lxorguk.ukuu.org.uk> 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.

^ permalink raw reply	[flat|nested] 48+ messages in thread

* Re: [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel
       [not found]       ` <20190318185742.109dee5c@alans-desktop>
  2019-03-18 19:11         ` Daniel Colascione
@ 2019-03-18 21:11         ` Karim Yaghmour
  1 sibling, 0 replies; 48+ messages in thread
From: Karim Yaghmour @ 2019-03-18 21:11 UTC (permalink / raw)
  To: Alan Cox, Greg KH
  Cc: Joel Fernandes, Geert Uytterhoeven, Linux Kernel Mailing List,
	Andrew Morton, Alexei Starovoitov, atishp04, dancol,
	Dan Williams, Dietmar Eggemann, Guenter Roeck, Jonathan Corbet,
	Kees Cook, Android Kernel Team, linux-doc, linux-kselftest,
	linux-trace-devel, Manoj Rao, Masahiro Yamada, Masami Hiramatsu,
	qais.yousef, Randy Dunlap, Steven Rostedt, Shuah Khan, yhs


Hi Alan,

On 3/18/19 2:57 PM, Alan Cox 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 ?

The Android use-case scenario might indeed have been the one to best 
crystallize the requirement, but that doesn't mean that all use-cases of 
eBPF wouldn't benefit from this -- which in fact they would, see 
instructions here for example on the need for kernel headers:
https://github.com/iovisor/bcc/blob/master/INSTALL.md

Just a quick $PREFERRED_SEARCH_ENGINE search returns interesting 
exchanges such as this one taken from a discussion thread on LWN 
covering an introduction to eBPF:
https://lwn.net/Articles/741348/
Effectively this person had to be hand-held in understanding that they 
needed a good set of kernel headers to make the tool work. Their comment 
after being shown that this was needed was:
"It looks like the headers package you need on Ubuntu is 
linux-headers-$(uname -r), which contains the entire kernel source tree, 
and is specific to the running kernel."

Surely having Joel's patch in the kernel would obviate the issue for all 
Linux kernel users, not just Android.

Cheers,

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

^ permalink raw reply	[flat|nested] 48+ messages in thread

end of thread, other threads:[~2019-03-18 21:11 UTC | newest]

Thread overview: 48+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-01 16:08 [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel Joel Fernandes (Google)
2019-03-01 16:08 ` [PATCH v4 2/2] Add selftests for module build using in-kernel headers Joel Fernandes (Google)
2019-03-02 21:59 ` [PATCH v4 1/2] Provide in-kernel headers for making it easy to extend the kernel kbuild test robot
2019-03-03 16:11   ` Joel Fernandes
2019-03-06 12:26     ` Masahiro Yamada
2019-03-06 17:49       ` Joel Fernandes
2019-03-07  4:59         ` Masahiro Yamada
2019-03-07 14:54           ` Joel Fernandes
2019-03-07 23:23       ` Justin Capella
2019-03-06 18:16     ` Joel Fernandes
2019-03-07  4:54       ` Masahiro Yamada
2019-03-03  2:04 ` kbuild test robot
2019-03-04 14:00 ` Qais Yousef
2019-03-05 16:27   ` Joel Fernandes
2019-03-04 22:48 ` Dietmar Eggemann
2019-03-05 16:25   ` Joel Fernandes
2019-03-07  8:58 ` Geert Uytterhoeven
2019-03-07 15:03   ` Joel Fernandes
2019-03-07 15:23     ` Greg KH
2019-03-07 16:54       ` Joel Fernandes
     [not found]       ` <20190318185742.109dee5c@alans-desktop>
2019-03-18 19:11         ` Daniel Colascione
2019-03-18 21:11         ` Karim Yaghmour
2019-03-08  8:53     ` Geert Uytterhoeven
2019-03-08 13:42       ` Joel Fernandes
2019-03-08 13:57         ` Enrico Weigelt, metux IT consult
2019-03-08 14:04           ` Greg KH
2019-03-08 14:02         ` Greg KH
2019-03-08 17:58           ` Joel Fernandes
2019-03-08 17:59           ` Geert Uytterhoeven
2019-03-09  7:16             ` Greg KH
2019-03-09 11:40               ` Geert Uytterhoeven
2019-03-09 12:11                 ` Greg KH
2019-03-09 16:51                   ` Karim Yaghmour
2019-03-09 19:26                     ` Geert Uytterhoeven
2019-03-09 21:44                       ` Karim Yaghmour
2019-03-11  8:03                         ` Geert Uytterhoeven
2019-03-12 15:15                           ` Karim Yaghmour
2019-03-11 23:36                         ` Steven Rostedt
2019-03-11 23:58                           ` Daniel Colascione
2019-03-12  0:39                             ` Joel Fernandes
2019-03-12  1:28                               ` Steven Rostedt
2019-03-12  1:38                                 ` Joel Fernandes
2019-03-13  1:18                                   ` Masami Hiramatsu
2019-03-14 12:27                                     ` Joel Fernandes
2019-03-15 13:14                                       ` Masami Hiramatsu
2019-03-12  1:45                                 ` Alexei Starovoitov
2019-03-12 15:26                                   ` Steven Rostedt
2019-03-12  1:22                             ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).