| Message ID | 20190408212855.233198-1-joel@joelfernandes.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v6,1/2] Provide in-kernel headers to make extending kernel easier | expand |
On 04/08/19 17:28, Joel Fernandes (Google) wrote: > Introduce in-kernel headers which are made available as an archive > through proc (/proc/kheaders.tar.xz file). This archive makes it > possible to run eBPF and other tracing programs tracing programs that > need to extend the kernel for tracing purposes without any dependency on > the file system having headers. > > On Android and embedded systems, it is common to switch kernels but not > have kernel headers available on the file system. Further once a > different kernel is booted, any headers stored on the file system will > no longer be useful. By storing the headers as a compressed archive > within the kernel, we can avoid these issues that have been a hindrance > for a long time. > > The best way to use this feature is by building it in. Several users > have a need for this, when they switch debug kernels, they donot want to > update the filesystem or worry about it where to store the headers on > it. However, the feature is also buildable as a module in case the user > desires it not being part of the kernel image. This makes it possible to > load and unload the headers from memory on demand. A tracing program, or > a kernel module builder can load the module, do its operations, and then > unload the module to save kernel memory. The total memory needed is 3.8MB. > > By having the archive available at a fixed location independent of > filesystem dependencies and conventions, all debugging tools can > directly refer to the fixed location for the archive, without concerning > with where the headers on a typical filesystem which significantly > simplifies tooling that needs kernel headers. > > The code to read the headers is based on /proc/config.gz code and uses > the same technique to embed the headers. > > IKHD_ST and IKHD_ED markers as is to facilitate future patches that > would extract the headers from a kernel or module image. > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> I applied both patches on 5.1-rc2 and managed to compile and run several eBPF programs using the untarred headers from /proc/kheaders.tar.xz on my juno. Tested-by: Qais Yousef <qais.yousef@arm.com> > --- > > v5 -> v6: (Masahiro Yamada suggestions mostly) > - Dropped support for module building. > - Rebuild archive if script changes. > - Move archive file list to script. > - Move build script to kernel directory. > > v4 -> v5: > (v4 was Tested-by the following folks) > Tested-by: qais.yousef@arm.com > Tested-by: dietmar.eggemann@arm.com > Tested-by: linux@manojrajarao.com Thanks! -- Qais Yousef
On Tue, Apr 09, 2019 at 04:00:34PM +0100, Qais Yousef wrote: > On 04/08/19 17:28, Joel Fernandes (Google) wrote: > > Introduce in-kernel headers which are made available as an archive > > through proc (/proc/kheaders.tar.xz file). This archive makes it > > possible to run eBPF and other tracing programs tracing programs that > > need to extend the kernel for tracing purposes without any dependency on > > the file system having headers. > > > > On Android and embedded systems, it is common to switch kernels but not > > have kernel headers available on the file system. Further once a > > different kernel is booted, any headers stored on the file system will > > no longer be useful. By storing the headers as a compressed archive > > within the kernel, we can avoid these issues that have been a hindrance > > for a long time. > > > > The best way to use this feature is by building it in. Several users > > have a need for this, when they switch debug kernels, they donot want to > > update the filesystem or worry about it where to store the headers on > > it. However, the feature is also buildable as a module in case the user > > desires it not being part of the kernel image. This makes it possible to > > load and unload the headers from memory on demand. A tracing program, or > > a kernel module builder can load the module, do its operations, and then > > unload the module to save kernel memory. The total memory needed is 3.8MB. > > > > By having the archive available at a fixed location independent of > > filesystem dependencies and conventions, all debugging tools can > > directly refer to the fixed location for the archive, without concerning > > with where the headers on a typical filesystem which significantly > > simplifies tooling that needs kernel headers. > > > > The code to read the headers is based on /proc/config.gz code and uses > > the same technique to embed the headers. > > > > IKHD_ST and IKHD_ED markers as is to facilitate future patches that > > would extract the headers from a kernel or module image. > > > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > I applied both patches on 5.1-rc2 and managed to compile and run several > eBPF programs using the untarred headers from /proc/kheaders.tar.xz on my juno. > > Tested-by: Qais Yousef <qais.yousef@arm.com> Thank you Qais! Masahiro, if Ok with you now could you give your tag? thanks, - Joel > > --- > > > > v5 -> v6: (Masahiro Yamada suggestions mostly) > > - Dropped support for module building. > > - Rebuild archive if script changes. > > - Move archive file list to script. > > - Move build script to kernel directory. > > > > v4 -> v5: > > (v4 was Tested-by the following folks) > > Tested-by: qais.yousef@arm.com > > Tested-by: dietmar.eggemann@arm.com > > Tested-by: linux@manojrajarao.com > > Thanks! > > -- > Qais Yousef
Hi Joel, I have no objection to this patch. I checked though it once again, please let me point out a little more. They are all nits. On Tue, Apr 9, 2019 at 6:37 AM Joel Fernandes (Google) <joel@joelfernandes.org> wrote: > > Introduce in-kernel headers which are made available as an archive > through proc (/proc/kheaders.tar.xz file). This archive makes it > possible to run eBPF and other tracing programs tracing programs that Just one "tracing programs" is enough. > need to extend the kernel for tracing purposes without any dependency on > the file system having headers. > > On Android and embedded systems, it is common to switch kernels but not > have kernel headers available on the file system. Further once a > different kernel is booted, any headers stored on the file system will > no longer be useful. By storing the headers as a compressed archive > within the kernel, we can avoid these issues that have been a hindrance > for a long time. > > The best way to use this feature is by building it in. Several users > have a need for this, when they switch debug kernels, they donot want to 'donot' -> 'do not' ? > update the filesystem or worry about it where to store the headers on > it. However, the feature is also buildable as a module in case the user > desires it not being part of the kernel image. This makes it possible to > load and unload the headers from memory on demand. A tracing program, or > a kernel module builder can load the module, do its operations, and then > unload the module to save kernel memory. The total memory needed is 3.8MB. > > By having the archive available at a fixed location independent of > filesystem dependencies and conventions, all debugging tools can > directly refer to the fixed location for the archive, without concerning > with where the headers on a typical filesystem which significantly > simplifies tooling that needs kernel headers. > > The code to read the headers is based on /proc/config.gz code and uses > the same technique to embed the headers. > > IKHD_ST and IKHD_ED markers as is to facilitate future patches that > would extract the headers from a kernel or module image. > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> [snip] > > diff --git a/init/Kconfig b/init/Kconfig > index 4592bf7997c0..ea75bfbf7dfa 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -580,6 +580,17 @@ config IKCONFIG_PROC > This option enables access to the kernel configuration file > through /proc/config.gz. > > +config IKHEADERS_PROC > + tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz" > + depends on PROC_FS > + help > + This option enables access to the kernel header and other artifacts that This line is indented by a TAB, which is correct. > + are generated during the build process. These can be used to build kernel > + modules or by other in-kernel programs such as those generated by eBPF Now that you have dropped the ability to "build kernel modules", I'd like you to update this help message. > + and systemtap tools. If you build the headers as a module, a module > + called kheaders.ko is built which can be loaded on-demand to get access > + to the headers. These lines are indented by 8-spaces instead of one TAB. Please use TAB-indentation consistently. [snip] > +rm -rf $cpio_dir > +mkdir $cpio_dir > + > +pushd $kroot > /dev/null > +for f in $src_file_list; > + do find "$f" ! -name "*.cmd" ! -name ".*"; > +done | cpio --quiet -pd $cpio_dir > +popd > /dev/null > + > +# The second CPIO can complain if files already exist which can > +# happen with out of tree builds. Just silence CPIO for now. > +for f in $obj_file_list; > + do find "$f" ! -name "*.cmd" ! -name ".*"; > +done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1 > + Could you add a simple comment about what the following code is doing? "Remove comments except SPDX" etc. > +find $cpio_dir -type f -print0 | Please replace two spaces after 'find' with one. > + xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;' > + > +tar -Jcf $tarfile -C $cpio_dir/ . > /dev/null > + > +echo "$src_files_md5" > kernel/kheaders.md5 > +echo "$obj_files_md5" >> kernel/kheaders.md5 > +echo "$(md5sum $tarfile | cut -d ' ' -f1)" >> kernel/kheaders.md5 > + > +rm -rf $cpio_dir > diff --git a/kernel/kheaders.c b/kernel/kheaders.c > new file mode 100644 > index 000000000000..d072a958a8f1 > --- /dev/null > +++ b/kernel/kheaders.c > @@ -0,0 +1,73 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * kernel/kheaders.c > + * Provide headers and artifacts needed to build kernel modules. Ditto. Could you update this comment ? > + * (Borrowed code from kernel/configs.c) > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/proc_fs.h> > +#include <linux/init.h> > +#include <linux/uaccess.h> > + > +/* > + * Define kernel_headers_data and kernel_headers_data_end, within which the the > + * compressed kernel headers are stpred. The file is first compressed with xz. > + */ > + > +asm ( > +" .pushsection .rodata, \"a\" \n" > +" .global kernel_headers_data \n" > +"kernel_headers_data: \n" > +" .incbin \"kernel/kheaders_data.tar.xz\" \n" > +" .global kernel_headers_data_end \n" > +"kernel_headers_data_end: \n" > +" .popsection \n" > +); You mentioned "IKHD_ST and IKHD_ED markers..." in the commit description, but I do not see them in the code. If you plan to work on a tool to extract the headers, I think it is OK to have the markers here. Anyway, please make the code and the commit-log consistent. > + > +extern char kernel_headers_data; > +extern char kernel_headers_data_end; > + > +static ssize_t > +ikheaders_read_current(struct file *file, char __user *buf, Could you stretch this line ? It will still fit in 80-cols. (This is a coding style error in kernel/configs.c) Last thing, when CONFIG_IKHEADERS_PROC=y, I always see: GEN kernel/kheaders_data.tar.xz which I think misleading because the script is just checking the md5sum. What I like to see is: CHK kernel/kheaders_data.tar.xz for checking md5sum. And, GEN kernel/kheaders_data.tar.xz for really (re-)generating the tarball. How about this code? index e3c581d8cde7..12399614c350 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -125,7 +125,7 @@ $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz -quiet_cmd_genikh = GEN $(obj)/kheaders_data.tar.xz +quiet_cmd_genikh = CHK $(obj)/kheaders_data.tar.xz cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@ $(obj)/kheaders_data.tar.xz: FORCE $(call cmd,genikh) diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_ikh_data.sh index ef72c2740d01..613960e18691 100755 --- a/kernel/gen_ikh_data.sh +++ b/kernel/gen_ikh_data.sh @@ -57,6 +57,10 @@ if [ -f kernel/kheaders.md5 ] && exit fi +if [ "${quiet}" != "silent_" ]; then + echo " GEN $tarfile" +fi + rm -rf $cpio_dir mkdir $cpio_dir Thanks.
On Thu, Apr 11, 2019 at 10:47:23AM +0900, Masahiro Yamada wrote: > Hi Joel, > > I have no objection to this patch. > I checked though it once again, > please let me point out a little more. > > They are all nits. > > > > On Tue, Apr 9, 2019 at 6:37 AM Joel Fernandes (Google) > <joel@joelfernandes.org> wrote: > > > > Introduce in-kernel headers which are made available as an archive > > through proc (/proc/kheaders.tar.xz file). This archive makes it > > possible to run eBPF and other tracing programs tracing programs that > > Just one "tracing programs" is enough. fixed. > > need to extend the kernel for tracing purposes without any dependency on > > the file system having headers. > > > > On Android and embedded systems, it is common to switch kernels but not > > have kernel headers available on the file system. Further once a > > different kernel is booted, any headers stored on the file system will > > no longer be useful. By storing the headers as a compressed archive > > within the kernel, we can avoid these issues that have been a hindrance > > for a long time. > > > > The best way to use this feature is by building it in. Several users > > have a need for this, when they switch debug kernels, they donot want to > > 'donot' -> 'do not' ? fixed [snip] > > > > diff --git a/init/Kconfig b/init/Kconfig > > index 4592bf7997c0..ea75bfbf7dfa 100644 > > --- a/init/Kconfig > > +++ b/init/Kconfig > > @@ -580,6 +580,17 @@ config IKCONFIG_PROC > > This option enables access to the kernel configuration file > > through /proc/config.gz. > > > > +config IKHEADERS_PROC > > + tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz" > > + depends on PROC_FS > > + help > > + This option enables access to the kernel header and other artifacts that > > This line is indented by a TAB, which is correct. > > > > + are generated during the build process. These can be used to build kernel > > + modules or by other in-kernel programs such as those generated by eBPF > > Now that you have dropped the ability to "build kernel modules", > I'd like you to update this help message. Sorry, will fix. > > + and systemtap tools. If you build the headers as a module, a module > > + called kheaders.ko is built which can be loaded on-demand to get access > > + to the headers. > > These lines are indented by 8-spaces instead of one TAB. > Please use TAB-indentation consistently. > > > [snip] > > > > +rm -rf $cpio_dir > > +mkdir $cpio_dir > > + > > +pushd $kroot > /dev/null > > +for f in $src_file_list; > > + do find "$f" ! -name "*.cmd" ! -name ".*"; > > +done | cpio --quiet -pd $cpio_dir > > +popd > /dev/null > > + > > +# The second CPIO can complain if files already exist which can > > +# happen with out of tree builds. Just silence CPIO for now. > > +for f in $obj_file_list; > > + do find "$f" ! -name "*.cmd" ! -name ".*"; > > +done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1 > > + > > Could you add a simple comment about what the following code is doing? > "Remove comments except SPDX" etc. Ok. > > +find $cpio_dir -type f -print0 | > > Please replace two spaces after 'find' with one. fixed > > + xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;' > > + > > +tar -Jcf $tarfile -C $cpio_dir/ . > /dev/null > > + > > +echo "$src_files_md5" > kernel/kheaders.md5 > > +echo "$obj_files_md5" >> kernel/kheaders.md5 > > +echo "$(md5sum $tarfile | cut -d ' ' -f1)" >> kernel/kheaders.md5 > > + > > +rm -rf $cpio_dir > > diff --git a/kernel/kheaders.c b/kernel/kheaders.c > > new file mode 100644 > > index 000000000000..d072a958a8f1 > > --- /dev/null > > +++ b/kernel/kheaders.c > > @@ -0,0 +1,73 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * kernel/kheaders.c > > + * Provide headers and artifacts needed to build kernel modules. > > Ditto. Could you update this comment ? fixed. > > + * (Borrowed code from kernel/configs.c) > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/module.h> > > +#include <linux/proc_fs.h> > > +#include <linux/init.h> > > +#include <linux/uaccess.h> > > + > > +/* > > + * Define kernel_headers_data and kernel_headers_data_end, within which the the > > + * compressed kernel headers are stpred. The file is first compressed with xz. > > + */ > > + > > +asm ( > > +" .pushsection .rodata, \"a\" \n" > > +" .global kernel_headers_data \n" > > +"kernel_headers_data: \n" > > +" .incbin \"kernel/kheaders_data.tar.xz\" \n" > > +" .global kernel_headers_data_end \n" > > +"kernel_headers_data_end: \n" > > +" .popsection \n" > > +); > > You mentioned "IKHD_ST and IKHD_ED markers..." in the commit description, > but I do not see them in the code. fixed > If you plan to work on a tool to extract the headers, > I think it is OK to have the markers here. > > Anyway, please make the code and the commit-log consistent. yes, will do. thanks > > +extern char kernel_headers_data; > > +extern char kernel_headers_data_end; > > + > > +static ssize_t > > +ikheaders_read_current(struct file *file, char __user *buf, > > Could you stretch this line ? > It will still fit in 80-cols. > > (This is a coding style error in kernel/configs.c) It takes 87 cols if I expand, so I'll leave it as is. > Last thing, when CONFIG_IKHEADERS_PROC=y, > I always see: > GEN kernel/kheaders_data.tar.xz > > which I think misleading because > the script is just checking the md5sum. > > > > What I like to see is: > CHK kernel/kheaders_data.tar.xz > for checking md5sum. > > And, > GEN kernel/kheaders_data.tar.xz > for really (re-)generating the tarball. > > > How about this code? Yes this is better, I changed it to that. Also looking forward to your tag on the v7 posting if it looks to you now. thanks a lot for the review! - Joel > index e3c581d8cde7..12399614c350 100644 > --- a/kernel/Makefile > +++ b/kernel/Makefile > @@ -125,7 +125,7 @@ $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE > > $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz > > -quiet_cmd_genikh = GEN $(obj)/kheaders_data.tar.xz > +quiet_cmd_genikh = CHK $(obj)/kheaders_data.tar.xz > cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@ > $(obj)/kheaders_data.tar.xz: FORCE > $(call cmd,genikh) > diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_ikh_data.sh > index ef72c2740d01..613960e18691 100755 > --- a/kernel/gen_ikh_data.sh > +++ b/kernel/gen_ikh_data.sh > @@ -57,6 +57,10 @@ if [ -f kernel/kheaders.md5 ] && > exit > fi > > +if [ "${quiet}" != "silent_" ]; then > + echo " GEN $tarfile" > +fi > + > rm -rf $cpio_dir > mkdir $cpio_dir > > > Thanks. > > -- > Best Regards > Masahiro Yamada
On Sat, Apr 13, 2019 at 3:02 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > +extern char kernel_headers_data; > > > +extern char kernel_headers_data_end; > > > + > > > +static ssize_t > > > +ikheaders_read_current(struct file *file, char __user *buf, > > > > Could you stretch this line ? > > It will still fit in 80-cols. > > > > (This is a coding style error in kernel/configs.c) > > It takes 87 cols if I expand, so I'll leave it as is. > Sorry if what I said was unclear. Since I just did not a good reason to put "static ssize_t" in the previous line, I meant like follows: [Before] static ssize_t ikheaders_read_current(struct file *file, char __user *buf, size_t len, loff_t *offset) [After] static ssize_t ikheaders_read_current(struct file *file, char __user *buf, size_t len, loff_t *offset) (takes 74-cols.) (I am sending this from Gmail, so I am not sure how it will look like from you...) Anyway, it is super-bikeshedding stuff. It is OK as-is. -- Best Regards Masahiro Yamada
On Fri, Apr 12, 2019 at 6:49 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Sat, Apr 13, 2019 at 3:02 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > > > +extern char kernel_headers_data; > > > > +extern char kernel_headers_data_end; > > > > + > > > > +static ssize_t > > > > +ikheaders_read_current(struct file *file, char __user *buf, > > > > > > Could you stretch this line ? > > > It will still fit in 80-cols. > > > > > > (This is a coding style error in kernel/configs.c) > > > > It takes 87 cols if I expand, so I'll leave it as is. > > > > > Sorry if what I said was unclear. > > Since I just did not a good reason to put > "static ssize_t" in the previous line, > I meant like follows: > > > [Before] > static ssize_t > ikheaders_read_current(struct file *file, char __user *buf, > size_t len, loff_t *offset) > > > > [After] > static ssize_t ikheaders_read_current(struct file *file, char __user *buf, > size_t len, loff_t *offset) > > > (takes 74-cols.) > > > (I am sending this from Gmail, so I am not sure > how it will look like from you...) > > > Anyway, it is super-bikeshedding stuff. > It is OK as-is. > What about sorting the files for determinism?
On Sat, Apr 13, 2019 at 10:53 AM Daniel Colascione <dancol@google.com> wrote: > > On Fri, Apr 12, 2019 at 6:49 PM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > On Sat, Apr 13, 2019 at 3:02 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > > > > > > +extern char kernel_headers_data; > > > > > +extern char kernel_headers_data_end; > > > > > + > > > > > +static ssize_t > > > > > +ikheaders_read_current(struct file *file, char __user *buf, > > > > > > > > Could you stretch this line ? > > > > It will still fit in 80-cols. > > > > > > > > (This is a coding style error in kernel/configs.c) > > > > > > It takes 87 cols if I expand, so I'll leave it as is. > > > > > > > > > Sorry if what I said was unclear. > > > > Since I just did not a good reason to put > > "static ssize_t" in the previous line, > > I meant like follows: > > > > > > [Before] > > static ssize_t > > ikheaders_read_current(struct file *file, char __user *buf, > > size_t len, loff_t *offset) > > > > > > > > [After] > > static ssize_t ikheaders_read_current(struct file *file, char __user *buf, > > size_t len, loff_t *offset) > > > > > > (takes 74-cols.) > > > > > > (I am sending this from Gmail, so I am not sure > > how it will look like from you...) > > > > > > Anyway, it is super-bikeshedding stuff. > > It is OK as-is. > > > > What about sorting the files for determinism? Do you mean the order of files in the tar archive? I think it is a good idea for reproducible build. It looks like the tar command supports --sort=name option, but I did not test it at all. Perhaps, the sorting policy could be affected by locale. We saw it for the 'find' command. I am not sure about the tar command though. commit f55f2328bb28a8517620518c5d124f5194673309 Author: Andrzej Pietrasiewicz <andrzej.p@samsung.com> Date: Fri Aug 17 14:03:19 2018 +0200 kbuild: make sorting initramfs contents independent of locale -- Best Regards Masahiro Yamada
On Sat, Apr 13, 2019 at 11:58 AM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Sat, Apr 13, 2019 at 10:53 AM Daniel Colascione <dancol@google.com> wrote: > > > > On Fri, Apr 12, 2019 at 6:49 PM Masahiro Yamada > > <yamada.masahiro@socionext.com> wrote: > > > > > > On Sat, Apr 13, 2019 at 3:02 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > > > > > > > > > +extern char kernel_headers_data; > > > > > > +extern char kernel_headers_data_end; > > > > > > + > > > > > > +static ssize_t > > > > > > +ikheaders_read_current(struct file *file, char __user *buf, > > > > > > > > > > Could you stretch this line ? > > > > > It will still fit in 80-cols. > > > > > > > > > > (This is a coding style error in kernel/configs.c) > > > > > > > > It takes 87 cols if I expand, so I'll leave it as is. > > > > > > > > > > > > > Sorry if what I said was unclear. > > > > > > Since I just did not a good reason to put > > > "static ssize_t" in the previous line, > > > I meant like follows: > > > > > > > > > [Before] > > > static ssize_t > > > ikheaders_read_current(struct file *file, char __user *buf, > > > size_t len, loff_t *offset) > > > > > > > > > > > > [After] > > > static ssize_t ikheaders_read_current(struct file *file, char __user *buf, > > > size_t len, loff_t *offset) > > > > > > > > > (takes 74-cols.) > > > > > > > > > (I am sending this from Gmail, so I am not sure > > > how it will look like from you...) > > > > > > > > > Anyway, it is super-bikeshedding stuff. > > > It is OK as-is. > > > > > > > What about sorting the files for determinism? > > > Do you mean the order of files in the tar archive? > I think it is a good idea for reproducible build. > > > > It looks like the tar command supports --sort=name option, > but I did not test it at all. > > > > Perhaps, the sorting policy could be affected by locale. > We saw it for the 'find' command. 'sort' command is affected by locale. > I am not sure about the tar command though. > > > > commit f55f2328bb28a8517620518c5d124f5194673309 > Author: Andrzej Pietrasiewicz <andrzej.p@samsung.com> > Date: Fri Aug 17 14:03:19 2018 +0200 > > kbuild: make sorting initramfs contents independent of locale > > > > > > > > > -- > Best Regards > Masahiro Yamada
On Fri, Apr 12, 2019 at 8:10 PM Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > > On Sat, Apr 13, 2019 at 11:58 AM Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: > > > > On Sat, Apr 13, 2019 at 10:53 AM Daniel Colascione <dancol@google.com> wrote: > > > > > > On Fri, Apr 12, 2019 at 6:49 PM Masahiro Yamada > > > <yamada.masahiro@socionext.com> wrote: > > > > > > > > On Sat, Apr 13, 2019 at 3:02 AM Joel Fernandes <joel@joelfernandes.org> wrote: > > > > > > > > > > > > > > > > +extern char kernel_headers_data; > > > > > > > +extern char kernel_headers_data_end; > > > > > > > + > > > > > > > +static ssize_t > > > > > > > +ikheaders_read_current(struct file *file, char __user *buf, > > > > > > > > > > > > Could you stretch this line ? > > > > > > It will still fit in 80-cols. > > > > > > > > > > > > (This is a coding style error in kernel/configs.c) > > > > > > > > > > It takes 87 cols if I expand, so I'll leave it as is. > > > > > > > > > > > > > > > > > Sorry if what I said was unclear. > > > > > > > > Since I just did not a good reason to put > > > > "static ssize_t" in the previous line, > > > > I meant like follows: > > > > > > > > > > > > [Before] > > > > static ssize_t > > > > ikheaders_read_current(struct file *file, char __user *buf, > > > > size_t len, loff_t *offset) > > > > > > > > > > > > > > > > [After] > > > > static ssize_t ikheaders_read_current(struct file *file, char __user *buf, > > > > size_t len, loff_t *offset) > > > > > > > > > > > > (takes 74-cols.) > > > > > > > > > > > > (I am sending this from Gmail, so I am not sure > > > > how it will look like from you...) > > > > > > > > > > > > Anyway, it is super-bikeshedding stuff. > > > > It is OK as-is. > > > > > > > > > > What about sorting the files for determinism? > > > > > > Do you mean the order of files in the tar archive? Yes. > > I think it is a good idea for reproducible build. > > > > > > > > It looks like the tar command supports --sort=name option, > > but I did not test it at all. > > > > > > > > Perhaps, the sorting policy could be affected by locale. > > We saw it for the 'find' command. > > > 'sort' command is affected by locale. It should be independent of locale if you set LC_ALL=C in the environment while running it.
diff --git a/init/Kconfig b/init/Kconfig index 4592bf7997c0..ea75bfbf7dfa 100644 --- a/init/Kconfig +++ b/init/Kconfig @@ -580,6 +580,17 @@ config IKCONFIG_PROC This option enables access to the kernel configuration file through /proc/config.gz. +config IKHEADERS_PROC + tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz" + depends on PROC_FS + help + This option enables access to the kernel header and other artifacts that + are generated during the build process. These can be used to build kernel + modules or by other in-kernel programs such as those generated by eBPF + and systemtap tools. If you build the headers as a module, a module + called kheaders.ko is built which can be loaded on-demand to get access + to the headers. + config LOG_BUF_SHIFT int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" range 12 25 diff --git a/kernel/.gitignore b/kernel/.gitignore index 6e699100872f..34d1e77ee9df 100644 --- a/kernel/.gitignore +++ b/kernel/.gitignore @@ -1,5 +1,6 @@ # # Generated files # +kheaders.md5 timeconst.h hz.bc diff --git a/kernel/Makefile b/kernel/Makefile index 6c57e78817da..e3c581d8cde7 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -70,6 +70,7 @@ obj-$(CONFIG_UTS_NS) += utsname.o obj-$(CONFIG_USER_NS) += user_namespace.o obj-$(CONFIG_PID_NS) += pid_namespace.o obj-$(CONFIG_IKCONFIG) += configs.o +obj-$(CONFIG_IKHEADERS_PROC) += kheaders.o obj-$(CONFIG_SMP) += stop_machine.o obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o obj-$(CONFIG_AUDIT) += audit.o auditfilter.o @@ -121,3 +122,12 @@ $(obj)/configs.o: $(obj)/config_data.gz targets += config_data.gz $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE $(call if_changed,gzip) + +$(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz + +quiet_cmd_genikh = GEN $(obj)/kheaders_data.tar.xz +cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@ +$(obj)/kheaders_data.tar.xz: FORCE + $(call cmd,genikh) + +clean-files := kheaders_data.tar.xz kheaders.md5 diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_ikh_data.sh new file mode 100755 index 000000000000..ef72c2740d01 --- /dev/null +++ b/kernel/gen_ikh_data.sh @@ -0,0 +1,84 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 + +# This script generates an archive consisting of kernel headers +# for CONFIG_IKHEADERS_PROC. +set -e +spath="$(dirname "$(readlink -f "$0")")" +kroot="$spath/.." +outdir="$(pwd)" +tarfile=$1 +cpio_dir=$outdir/$tarfile.tmp + +# Script filename relative to the kernel source root +# We add it to the archive because it is small and any changes +# to this script will also cause a rebuild of the archive. +sfile="$(realpath --relative-to $kroot "$(readlink -f "$0")")" + +src_file_list=" +include/ +arch/$SRCARCH/include/ +$sfile +" + +obj_file_list=" +include/ +arch/$SRCARCH/include/ +" + +# Support incremental builds by skipping archive generation +# if timestamps of files being archived are not changed. + +# This block is useful for debugging the incremental builds. +# Uncomment it for debugging. +# iter=1 +# if [ ! -f /tmp/iter ]; then echo 1 > /tmp/iter; +# else; iter=$(($(cat /tmp/iter) + 1)); fi +# find $src_file_list -type f | xargs ls -lR > /tmp/src-ls-$iter +# find $obj_file_list -type f | xargs ls -lR > /tmp/obj-ls-$iter + +# include/generated/compile.h is ignored because it is touched even when none +# of the source files changed. This causes pointless regeneration, so let us +# ignore them for md5 calculation. +pushd $kroot > /dev/null +src_files_md5="$(find $src_file_list -type f | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" +popd > /dev/null +obj_files_md5="$(find $obj_file_list -type f | + grep -v "include/generated/compile.h" | + xargs ls -lR | md5sum | cut -d ' ' -f1)" + +if [ -f $tarfile ]; then tarfile_md5="$(md5sum $tarfile | cut -d ' ' -f1)"; fi +if [ -f kernel/kheaders.md5 ] && + [ "$(cat kernel/kheaders.md5|head -1)" == "$src_files_md5" ] && + [ "$(cat kernel/kheaders.md5|head -2|tail -1)" == "$obj_files_md5" ] && + [ "$(cat kernel/kheaders.md5|tail -1)" == "$tarfile_md5" ]; then + exit +fi + +rm -rf $cpio_dir +mkdir $cpio_dir + +pushd $kroot > /dev/null +for f in $src_file_list; + do find "$f" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir +popd > /dev/null + +# The second CPIO can complain if files already exist which can +# happen with out of tree builds. Just silence CPIO for now. +for f in $obj_file_list; + do find "$f" ! -name "*.cmd" ! -name ".*"; +done | cpio --quiet -pd $cpio_dir >/dev/null 2>&1 + +find $cpio_dir -type f -print0 | + xargs -0 -P8 -n1 perl -pi -e 'BEGIN {undef $/;}; s/\/\*((?!SPDX).)*?\*\///smg;' + +tar -Jcf $tarfile -C $cpio_dir/ . > /dev/null + +echo "$src_files_md5" > kernel/kheaders.md5 +echo "$obj_files_md5" >> kernel/kheaders.md5 +echo "$(md5sum $tarfile | cut -d ' ' -f1)" >> kernel/kheaders.md5 + +rm -rf $cpio_dir diff --git a/kernel/kheaders.c b/kernel/kheaders.c new file mode 100644 index 000000000000..d072a958a8f1 --- /dev/null +++ b/kernel/kheaders.c @@ -0,0 +1,73 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * kernel/kheaders.c + * Provide headers and artifacts needed to build kernel modules. + * (Borrowed code from kernel/configs.c) + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/proc_fs.h> +#include <linux/init.h> +#include <linux/uaccess.h> + +/* + * Define kernel_headers_data and kernel_headers_data_end, within which the the + * compressed kernel headers are stpred. The file is first compressed with xz. + */ + +asm ( +" .pushsection .rodata, \"a\" \n" +" .global kernel_headers_data \n" +"kernel_headers_data: \n" +" .incbin \"kernel/kheaders_data.tar.xz\" \n" +" .global kernel_headers_data_end \n" +"kernel_headers_data_end: \n" +" .popsection \n" +); + +extern char kernel_headers_data; +extern char kernel_headers_data_end; + +static ssize_t +ikheaders_read_current(struct file *file, char __user *buf, + size_t len, loff_t *offset) +{ + return simple_read_from_buffer(buf, len, offset, + &kernel_headers_data, + &kernel_headers_data_end - + &kernel_headers_data); +} + +static const struct file_operations ikheaders_file_ops = { + .read = ikheaders_read_current, + .llseek = default_llseek, +}; + +static int __init ikheaders_init(void) +{ + struct proc_dir_entry *entry; + + /* create the current headers file */ + entry = proc_create("kheaders.tar.xz", S_IRUGO, NULL, + &ikheaders_file_ops); + if (!entry) + return -ENOMEM; + + proc_set_size(entry, + &kernel_headers_data_end - + &kernel_headers_data); + return 0; +} + +static void __exit ikheaders_cleanup(void) +{ + remove_proc_entry("kheaders.tar.xz", NULL); +} + +module_init(ikheaders_init); +module_exit(ikheaders_cleanup); + +MODULE_LICENSE("GPL v2"); +MODULE_AUTHOR("Joel Fernandes"); +MODULE_DESCRIPTION("Echo the kernel header artifacts used to build the kernel");