[PATCH v2] kheaders: Move from proc to sysfs

[PATCH v2] kheaders: Move from proc to sysfs

[Joel Fernandes (Google)]

The kheaders archive consisting of the kernel headers used for compiling
bpf programs is in /proc. However there is concern that moving it here
will make it permanent. Let us move it to /sys/kernel as discussed [1].

[1] https://lore.kernel.org/patchwork/patch/1067310/#1265969

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
This patch applies on top of the previous patch that was applied to the
driver tree:
https://lore.kernel.org/patchwork/patch/1067310/

v1->v2: Fixed some kconfig nits (Masami).

 init/Kconfig                                | 16 ++++-----
 kernel/Makefile                             |  4 +--
 kernel/{gen_ikh_data.sh => gen_kheaders.sh} |  2 +-
 kernel/kheaders.c                           | 40 +++++++++------------
 4 files changed, 26 insertions(+), 36 deletions(-)
 rename kernel/{gen_ikh_data.sh => gen_kheaders.sh} (98%)

diff --git a/init/Kconfig b/init/Kconfig
index 26a364a95b57..c3661991b089 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -579,15 +579,13 @@ config IKCONFIG_PROC
 	  This option enables access to the kernel configuration file
 	  through /proc/config.gz.
 
-config IKHEADERS_PROC
-	tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz"
-	depends on PROC_FS
-	help
-	  This option enables access to the kernel header and other artifacts that
-	  are generated during the build process. These can be used to build eBPF
-	  tracing programs, or similar programs.  If you build the headers as a
-	  module, a module called kheaders.ko is built which can be loaded on-demand
-	  to get access to the headers.
+config IKHEADERS
+	tristate "Enable kernel headers through /sys/kernel/kheaders.tar.xz"
+	help
+	  This option enables access to the in-kernel headers that are generated during
+	  the build process. These can be used to build eBPF tracing programs,
+	  or similar programs.  If you build the headers as a module, a module called
+	  kheaders.ko is built which can be loaded on-demand to get access to headers.
 
 config LOG_BUF_SHIFT
 	int "Kernel log buffer size (16 => 64KB, 17 => 128KB)"
diff --git a/kernel/Makefile b/kernel/Makefile
index 12399614c350..b32a558fae2f 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -70,7 +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_IKHEADERS) += kheaders.o
 obj-$(CONFIG_SMP) += stop_machine.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
@@ -126,7 +126,7 @@ $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
 $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
 
 quiet_cmd_genikh = CHK     $(obj)/kheaders_data.tar.xz
-cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@
+cmd_genikh = $(srctree)/kernel/gen_kheaders.sh $@
 $(obj)/kheaders_data.tar.xz: FORCE
 	$(call cmd,genikh)
 
diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_kheaders.sh
similarity index 98%
rename from kernel/gen_ikh_data.sh
rename to kernel/gen_kheaders.sh
index 591a94f7b387..581b83534587 100755
--- a/kernel/gen_ikh_data.sh
+++ b/kernel/gen_kheaders.sh
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 # This script generates an archive consisting of kernel headers
-# for CONFIG_IKHEADERS_PROC.
+# for CONFIG_IKHEADERS.
 set -e
 spath="$(dirname "$(readlink -f "$0")")"
 kroot="$spath/.."
diff --git a/kernel/kheaders.c b/kernel/kheaders.c
index 70ae6052920d..6a16f8f6898d 100644
--- a/kernel/kheaders.c
+++ b/kernel/kheaders.c
@@ -8,9 +8,8 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/proc_fs.h>
+#include <linux/kobject.h>
 #include <linux/init.h>
-#include <linux/uaccess.h>
 
 /*
  * Define kernel_headers_data and kernel_headers_data_end, within which the
@@ -31,39 +30,32 @@ 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)
+ikheaders_read(struct file *file,  struct kobject *kobj,
+	       struct bin_attribute *bin_attr,
+	       char *buf, loff_t off, size_t len)
 {
-	return simple_read_from_buffer(buf, len, offset,
-				       &kernel_headers_data,
-				       &kernel_headers_data_end -
-				       &kernel_headers_data);
+	memcpy(buf, &kernel_headers_data + off, len);
+	return len;
 }
 
-static const struct file_operations ikheaders_file_ops = {
-	.read = ikheaders_read_current,
-	.llseek = default_llseek,
+static struct bin_attribute kheaders_attr __ro_after_init = {
+	.attr = {
+		.name = "kheaders.tar.xz",
+		.mode = S_IRUGO,
+	},
+	.read = &ikheaders_read,
 };
 
 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;
+	kheaders_attr.size = (&kernel_headers_data_end -
+			      &kernel_headers_data);
+	return sysfs_create_bin_file(kernel_kobj, &kheaders_attr);
 }
 
 static void __exit ikheaders_cleanup(void)
 {
-	remove_proc_entry("kheaders.tar.xz", NULL);
+	sysfs_remove_bin_file(kernel_kobj, &kheaders_attr);
 }
 
 module_init(ikheaders_init);
-- 
2.21.0.1020.gf2820cf01a-goog

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Greg KH]

On Sat, May 04, 2019 at 08:12:13AM -0400, Joel Fernandes (Google) wrote:
> The kheaders archive consisting of the kernel headers used for compiling
> bpf programs is in /proc. However there is concern that moving it here
> will make it permanent. Let us move it to /sys/kernel as discussed [1].
> 
> [1] https://lore.kernel.org/patchwork/patch/1067310/#1265969
> 
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> This patch applies on top of the previous patch that was applied to the
> driver tree:
> https://lore.kernel.org/patchwork/patch/1067310/
> 
> v1->v2: Fixed some kconfig nits (Masami).
> 
>  init/Kconfig                                | 16 ++++-----
>  kernel/Makefile                             |  4 +--
>  kernel/{gen_ikh_data.sh => gen_kheaders.sh} |  2 +-
>  kernel/kheaders.c                           | 40 +++++++++------------
>  4 files changed, 26 insertions(+), 36 deletions(-)
>  rename kernel/{gen_ikh_data.sh => gen_kheaders.sh} (98%)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 26a364a95b57..c3661991b089 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -579,15 +579,13 @@ config IKCONFIG_PROC
>  	  This option enables access to the kernel configuration file
>  	  through /proc/config.gz.
>  
> -config IKHEADERS_PROC
> -	tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz"
> -	depends on PROC_FS
> -	help
> -	  This option enables access to the kernel header and other artifacts that
> -	  are generated during the build process. These can be used to build eBPF
> -	  tracing programs, or similar programs.  If you build the headers as a
> -	  module, a module called kheaders.ko is built which can be loaded on-demand
> -	  to get access to the headers.
> +config IKHEADERS
> +	tristate "Enable kernel headers through /sys/kernel/kheaders.tar.xz"
> +	help
> +	  This option enables access to the in-kernel headers that are generated during
> +	  the build process. These can be used to build eBPF tracing programs,
> +	  or similar programs.  If you build the headers as a module, a module called
> +	  kheaders.ko is built which can be loaded on-demand to get access to headers.
>  
>  config LOG_BUF_SHIFT
>  	int "Kernel log buffer size (16 => 64KB, 17 => 128KB)"
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 12399614c350..b32a558fae2f 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -70,7 +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_IKHEADERS) += kheaders.o

Good in changing the config name, I hadn't thought of that.

> diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_kheaders.sh
> similarity index 98%
> rename from kernel/gen_ikh_data.sh
> rename to kernel/gen_kheaders.sh

Same here, nice.

> -static const struct file_operations ikheaders_file_ops = {
> -	.read = ikheaders_read_current,
> -	.llseek = default_llseek,
> +static struct bin_attribute kheaders_attr __ro_after_init = {

Minor nit, are you sure about __ro_after_init for an attribute
structure?  Is it even needed?

> +	.attr = {
> +		.name = "kheaders.tar.xz",
> +		.mode = S_IRUGO,
> +	},
> +	.read = &ikheaders_read,
>  };

BIN_ATTR_RO?  Ah, no, that will not work with the name of the file,
nevermind.

But, you should change S_IRUGO to the correct octal number, checkpatch
should have barfed on this change.

thanks,

greg k-h

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Joel Fernandes]

On Sat, May 04, 2019 at 02:21:58PM +0200, Greg KH wrote:
> On Sat, May 04, 2019 at 08:12:13AM -0400, Joel Fernandes (Google) wrote:
> > The kheaders archive consisting of the kernel headers used for compiling
> > bpf programs is in /proc. However there is concern that moving it here
> > will make it permanent. Let us move it to /sys/kernel as discussed [1].
> > 
> > [1] https://lore.kernel.org/patchwork/patch/1067310/#1265969
> > 
> > Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> > ---
> > This patch applies on top of the previous patch that was applied to the
> > driver tree:
> > https://lore.kernel.org/patchwork/patch/1067310/
> > 
> > v1->v2: Fixed some kconfig nits (Masami).
> > 
> >  init/Kconfig                                | 16 ++++-----
> >  kernel/Makefile                             |  4 +--
> >  kernel/{gen_ikh_data.sh => gen_kheaders.sh} |  2 +-
> >  kernel/kheaders.c                           | 40 +++++++++------------
> >  4 files changed, 26 insertions(+), 36 deletions(-)
> >  rename kernel/{gen_ikh_data.sh => gen_kheaders.sh} (98%)
> > 
> > diff --git a/init/Kconfig b/init/Kconfig
> > index 26a364a95b57..c3661991b089 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -579,15 +579,13 @@ config IKCONFIG_PROC
> >  	  This option enables access to the kernel configuration file
> >  	  through /proc/config.gz.
> >  
> > -config IKHEADERS_PROC
> > -	tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz"
> > -	depends on PROC_FS
> > -	help
> > -	  This option enables access to the kernel header and other artifacts that
> > -	  are generated during the build process. These can be used to build eBPF
> > -	  tracing programs, or similar programs.  If you build the headers as a
> > -	  module, a module called kheaders.ko is built which can be loaded on-demand
> > -	  to get access to the headers.
> > +config IKHEADERS
> > +	tristate "Enable kernel headers through /sys/kernel/kheaders.tar.xz"
> > +	help
> > +	  This option enables access to the in-kernel headers that are generated during
> > +	  the build process. These can be used to build eBPF tracing programs,
> > +	  or similar programs.  If you build the headers as a module, a module called
> > +	  kheaders.ko is built which can be loaded on-demand to get access to headers.
> >  
> >  config LOG_BUF_SHIFT
> >  	int "Kernel log buffer size (16 => 64KB, 17 => 128KB)"
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index 12399614c350..b32a558fae2f 100644
> > --- a/kernel/Makefile
> > +++ b/kernel/Makefile
> > @@ -70,7 +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_IKHEADERS) += kheaders.o
> 
> Good in changing the config name, I hadn't thought of that.
> 
> > diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_kheaders.sh
> > similarity index 98%
> > rename from kernel/gen_ikh_data.sh
> > rename to kernel/gen_kheaders.sh
> 
> Same here, nice.

Thanks.

> > -static const struct file_operations ikheaders_file_ops = {
> > -	.read = ikheaders_read_current,
> > -	.llseek = default_llseek,
> > +static struct bin_attribute kheaders_attr __ro_after_init = {
> 
> Minor nit, are you sure about __ro_after_init for an attribute
> structure?  Is it even needed?

Yes, because after init, we don't need to change it, so it makes it more
robust to accidental overwrite. We change it on init because we have to
calculate the size of the archive. I was following this from kernel/ksysfs.c
actually.

I have recently encountered this mechanism in the module loader as well while
doing an RCU patch. The module loader post init will mark the page as
read-only and any writes to it will fault. These variables will be placed in
a ".data.ro_after_init" or a similarly named section in the module ELF.

> But, you should change S_IRUGO to the correct octal number, checkpatch
> should have barfed on this change.

fixed, below is the updated patch inline, thanks!

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

From: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Subject: [PATCH v3] kheaders: Move from proc to sysfs

The kheaders archive consisting of the kernel headers used for compiling
bpf programs is in /proc. However there is concern that moving it here
will make it permanent. Let us move it to /sys/kernel as discussed [1].

[1] https://lore.kernel.org/patchwork/patch/1067310/#1265969

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
This patch applies on top of the previous patch that was applied to the
driver tree:
https://lore.kernel.org/patchwork/patch/1067310/

v2->v3: Fixed sysfs file mode nit (Greg).
v1->v2: Fixed some kconfig nits (Masami).

Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
---
 init/Kconfig                                | 16 ++++-----
 kernel/Makefile                             |  4 +--
 kernel/{gen_ikh_data.sh => gen_kheaders.sh} |  2 +-
 kernel/kheaders.c                           | 40 +++++++++------------
 4 files changed, 26 insertions(+), 36 deletions(-)
 rename kernel/{gen_ikh_data.sh => gen_kheaders.sh} (98%)

diff --git a/init/Kconfig b/init/Kconfig
index 26a364a95b57..c3661991b089 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -579,15 +579,13 @@ config IKCONFIG_PROC
 	  This option enables access to the kernel configuration file
 	  through /proc/config.gz.
 
-config IKHEADERS_PROC
-	tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz"
-	depends on PROC_FS
-	help
-	  This option enables access to the kernel header and other artifacts that
-	  are generated during the build process. These can be used to build eBPF
-	  tracing programs, or similar programs.  If you build the headers as a
-	  module, a module called kheaders.ko is built which can be loaded on-demand
-	  to get access to the headers.
+config IKHEADERS
+	tristate "Enable kernel headers through /sys/kernel/kheaders.tar.xz"
+	help
+	  This option enables access to the in-kernel headers that are generated during
+	  the build process. These can be used to build eBPF tracing programs,
+	  or similar programs.  If you build the headers as a module, a module called
+	  kheaders.ko is built which can be loaded on-demand to get access to headers.
 
 config LOG_BUF_SHIFT
 	int "Kernel log buffer size (16 => 64KB, 17 => 128KB)"
diff --git a/kernel/Makefile b/kernel/Makefile
index 12399614c350..b32a558fae2f 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -70,7 +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_IKHEADERS) += kheaders.o
 obj-$(CONFIG_SMP) += stop_machine.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
@@ -126,7 +126,7 @@ $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
 $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
 
 quiet_cmd_genikh = CHK     $(obj)/kheaders_data.tar.xz
-cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@
+cmd_genikh = $(srctree)/kernel/gen_kheaders.sh $@
 $(obj)/kheaders_data.tar.xz: FORCE
 	$(call cmd,genikh)
 
diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_kheaders.sh
similarity index 98%
rename from kernel/gen_ikh_data.sh
rename to kernel/gen_kheaders.sh
index 591a94f7b387..581b83534587 100755
--- a/kernel/gen_ikh_data.sh
+++ b/kernel/gen_kheaders.sh
@@ -2,7 +2,7 @@
 # SPDX-License-Identifier: GPL-2.0
 
 # This script generates an archive consisting of kernel headers
-# for CONFIG_IKHEADERS_PROC.
+# for CONFIG_IKHEADERS.
 set -e
 spath="$(dirname "$(readlink -f "$0")")"
 kroot="$spath/.."
diff --git a/kernel/kheaders.c b/kernel/kheaders.c
index 70ae6052920d..8f69772af77b 100644
--- a/kernel/kheaders.c
+++ b/kernel/kheaders.c
@@ -8,9 +8,8 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/proc_fs.h>
+#include <linux/kobject.h>
 #include <linux/init.h>
-#include <linux/uaccess.h>
 
 /*
  * Define kernel_headers_data and kernel_headers_data_end, within which the
@@ -31,39 +30,32 @@ 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)
+ikheaders_read(struct file *file,  struct kobject *kobj,
+	       struct bin_attribute *bin_attr,
+	       char *buf, loff_t off, size_t len)
 {
-	return simple_read_from_buffer(buf, len, offset,
-				       &kernel_headers_data,
-				       &kernel_headers_data_end -
-				       &kernel_headers_data);
+	memcpy(buf, &kernel_headers_data + off, len);
+	return len;
 }
 
-static const struct file_operations ikheaders_file_ops = {
-	.read = ikheaders_read_current,
-	.llseek = default_llseek,
+static struct bin_attribute kheaders_attr __ro_after_init = {
+	.attr = {
+		.name = "kheaders.tar.xz",
+		.mode = 0444,
+	},
+	.read = &ikheaders_read,
 };
 
 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;
+	kheaders_attr.size = (&kernel_headers_data_end -
+			      &kernel_headers_data);
+	return sysfs_create_bin_file(kernel_kobj, &kheaders_attr);
 }
 
 static void __exit ikheaders_cleanup(void)
 {
-	remove_proc_entry("kheaders.tar.xz", NULL);
+	sysfs_remove_bin_file(kernel_kobj, &kheaders_attr);
 }
 
 module_init(ikheaders_init);
-- 
2.21.0.1020.gf2820cf01a-goog

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Masahiro Yamada]

On Sat, May 4, 2019 at 9:26 PM Joel Fernandes (Google)
<joel@joelfernandes.org> wrote:
>
> The kheaders archive consisting of the kernel headers used for compiling
> bpf programs is in /proc. However there is concern that moving it here
> will make it permanent. Let us move it to /sys/kernel as discussed [1].
>
> [1] https://lore.kernel.org/patchwork/patch/1067310/#1265969
>
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> ---
> This patch applies on top of the previous patch that was applied to the
> driver tree:
> https://lore.kernel.org/patchwork/patch/1067310/
>
> v1->v2: Fixed some kconfig nits (Masami).


My name is Masahiro, just in case.


>
>  init/Kconfig                                | 16 ++++-----
>  kernel/Makefile                             |  4 +--
>  kernel/{gen_ikh_data.sh => gen_kheaders.sh} |  2 +-
>  kernel/kheaders.c                           | 40 +++++++++------------
>  4 files changed, 26 insertions(+), 36 deletions(-)
>  rename kernel/{gen_ikh_data.sh => gen_kheaders.sh} (98%)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 26a364a95b57..c3661991b089 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -579,15 +579,13 @@ config IKCONFIG_PROC
>           This option enables access to the kernel configuration file
>           through /proc/config.gz.
>
> -config IKHEADERS_PROC
> -       tristate "Enable kernel header artifacts through /proc/kheaders.tar.xz"
> -       depends on PROC_FS
> -       help
> -         This option enables access to the kernel header and other artifacts that
> -         are generated during the build process. These can be used to build eBPF
> -         tracing programs, or similar programs.  If you build the headers as a
> -         module, a module called kheaders.ko is built which can be loaded on-demand
> -         to get access to the headers.
> +config IKHEADERS
> +       tristate "Enable kernel headers through /sys/kernel/kheaders.tar.xz"


In v1, I suggested to change "depends on PROC_FS"
to "depends on SYSFS".

fs/sysfs/ must be compiled to make this feature work.




> +       help
> +         This option enables access to the in-kernel headers that are generated during
> +         the build process. These can be used to build eBPF tracing programs,
> +         or similar programs.  If you build the headers as a module, a module called


While you are touching the help,
could you make the number of spaces
between sentences consistent?


Thanks.



> +         kheaders.ko is built which can be loaded on-demand to get access to headers.
>
>  config LOG_BUF_SHIFT
>         int "Kernel log buffer size (16 => 64KB, 17 => 128KB)"
> diff --git a/kernel/Makefile b/kernel/Makefile
> index 12399614c350..b32a558fae2f 100644
> --- a/kernel/Makefile
> +++ b/kernel/Makefile
> @@ -70,7 +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_IKHEADERS) += kheaders.o
>  obj-$(CONFIG_SMP) += stop_machine.o
>  obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
>  obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
> @@ -126,7 +126,7 @@ $(obj)/config_data.gz: $(KCONFIG_CONFIG) FORCE
>  $(obj)/kheaders.o: $(obj)/kheaders_data.tar.xz
>
>  quiet_cmd_genikh = CHK     $(obj)/kheaders_data.tar.xz
> -cmd_genikh = $(srctree)/kernel/gen_ikh_data.sh $@
> +cmd_genikh = $(srctree)/kernel/gen_kheaders.sh $@
>  $(obj)/kheaders_data.tar.xz: FORCE
>         $(call cmd,genikh)
>
> diff --git a/kernel/gen_ikh_data.sh b/kernel/gen_kheaders.sh
> similarity index 98%
> rename from kernel/gen_ikh_data.sh
> rename to kernel/gen_kheaders.sh
> index 591a94f7b387..581b83534587 100755
> --- a/kernel/gen_ikh_data.sh
> +++ b/kernel/gen_kheaders.sh
> @@ -2,7 +2,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>
>  # This script generates an archive consisting of kernel headers
> -# for CONFIG_IKHEADERS_PROC.
> +# for CONFIG_IKHEADERS.
>  set -e
>  spath="$(dirname "$(readlink -f "$0")")"
>  kroot="$spath/.."
> diff --git a/kernel/kheaders.c b/kernel/kheaders.c
> index 70ae6052920d..6a16f8f6898d 100644
> --- a/kernel/kheaders.c
> +++ b/kernel/kheaders.c
> @@ -8,9 +8,8 @@
>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> -#include <linux/proc_fs.h>
> +#include <linux/kobject.h>
>  #include <linux/init.h>
> -#include <linux/uaccess.h>
>
>  /*
>   * Define kernel_headers_data and kernel_headers_data_end, within which the
> @@ -31,39 +30,32 @@ 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)
> +ikheaders_read(struct file *file,  struct kobject *kobj,
> +              struct bin_attribute *bin_attr,
> +              char *buf, loff_t off, size_t len)
>  {
> -       return simple_read_from_buffer(buf, len, offset,
> -                                      &kernel_headers_data,
> -                                      &kernel_headers_data_end -
> -                                      &kernel_headers_data);
> +       memcpy(buf, &kernel_headers_data + off, len);
> +       return len;
>  }
>
> -static const struct file_operations ikheaders_file_ops = {
> -       .read = ikheaders_read_current,
> -       .llseek = default_llseek,
> +static struct bin_attribute kheaders_attr __ro_after_init = {
> +       .attr = {
> +               .name = "kheaders.tar.xz",
> +               .mode = S_IRUGO,
> +       },
> +       .read = &ikheaders_read,
>  };
>
>  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;
> +       kheaders_attr.size = (&kernel_headers_data_end -
> +                             &kernel_headers_data);
> +       return sysfs_create_bin_file(kernel_kobj, &kheaders_attr);
>  }
>
>  static void __exit ikheaders_cleanup(void)
>  {
> -       remove_proc_entry("kheaders.tar.xz", NULL);
> +       sysfs_remove_bin_file(kernel_kobj, &kheaders_attr);
>  }
>
>  module_init(ikheaders_init);
> --
> 2.21.0.1020.gf2820cf01a-goog



--
Best Regards

Masahiro Yamada

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Greg KH]

On Sat, May 04, 2019 at 08:36:50AM -0400, Joel Fernandes wrote:
> > But, you should change S_IRUGO to the correct octal number, checkpatch
> > should have barfed on this change.
> 
> fixed, below is the updated patch inline, thanks!

Please resend as a "real" submission, doing so in this format is a bit
more difficult to apply.

thanks,

greg k-h

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Joel Fernandes]

On Sun, May 05, 2019 at 11:10:30AM +0200, Greg KH wrote:
> On Sat, May 04, 2019 at 08:36:50AM -0400, Joel Fernandes wrote:
> > > But, you should change S_IRUGO to the correct octal number, checkpatch
> > > should have barfed on this change.
> > 
> > fixed, below is the updated patch inline, thanks!
> 
> Please resend as a "real" submission, doing so in this format is a bit
> more difficult to apply.

git am --scissors can do it, but no problem I will send as a formal
submission. Thanks a lot.


> 
> thanks,
> 
> greg k-h

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Steven Rostedt]

On Sun, 5 May 2019 13:26:23 +0000
Joel Fernandes <joel@joelfernandes.org> wrote:

> On Sun, May 05, 2019 at 11:10:30AM +0200, Greg KH wrote:
> > On Sat, May 04, 2019 at 08:36:50AM -0400, Joel Fernandes wrote:  
> > > > But, you should change S_IRUGO to the correct octal number, checkpatch
> > > > should have barfed on this change.  
> > > 
> > > fixed, below is the updated patch inline, thanks!  
> > 
> > Please resend as a "real" submission, doing so in this format is a bit
> > more difficult to apply.  
> 
> git am --scissors can do it, but no problem I will send as a formal
> submission. Thanks a lot.
>

True, but a lot of us depend on scripts to pull in patches from our
INBOX. Which is why we like them to stay with the standard format.

-- Steve

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Joel Fernandes]

On Sun, May 05, 2019 at 04:31:45PM -0400, Steven Rostedt wrote:
> On Sun, 5 May 2019 13:26:23 +0000
> Joel Fernandes <joel@joelfernandes.org> wrote:
> 
> > On Sun, May 05, 2019 at 11:10:30AM +0200, Greg KH wrote:
> > > On Sat, May 04, 2019 at 08:36:50AM -0400, Joel Fernandes wrote:  
> > > > > But, you should change S_IRUGO to the correct octal number, checkpatch
> > > > > should have barfed on this change.  
> > > > 
> > > > fixed, below is the updated patch inline, thanks!  
> > > 
> > > Please resend as a "real" submission, doing so in this format is a bit
> > > more difficult to apply.  
> > 
> > git am --scissors can do it, but no problem I will send as a formal
> > submission. Thanks a lot.
> >
> 
> True, but a lot of us depend on scripts to pull in patches from our
> INBOX. Which is why we like them to stay with the standard format.

Thanks makes sense. So Greg, I submitted this properly, does it look good to
you now? Steven, I would appreciate any Acks/Reviews on the patch as well:
https://lore.kernel.org/patchwork/patch/1070199/

- Joel

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Greg KH]

On Tue, May 07, 2019 at 12:38:24PM -0400, Joel Fernandes wrote:
> On Sun, May 05, 2019 at 04:31:45PM -0400, Steven Rostedt wrote:
> > On Sun, 5 May 2019 13:26:23 +0000
> > Joel Fernandes <joel@joelfernandes.org> wrote:
> > 
> > > On Sun, May 05, 2019 at 11:10:30AM +0200, Greg KH wrote:
> > > > On Sat, May 04, 2019 at 08:36:50AM -0400, Joel Fernandes wrote:  
> > > > > > But, you should change S_IRUGO to the correct octal number, checkpatch
> > > > > > should have barfed on this change.  
> > > > > 
> > > > > fixed, below is the updated patch inline, thanks!  
> > > > 
> > > > Please resend as a "real" submission, doing so in this format is a bit
> > > > more difficult to apply.  
> > > 
> > > git am --scissors can do it, but no problem I will send as a formal
> > > submission. Thanks a lot.
> > >
> > 
> > True, but a lot of us depend on scripts to pull in patches from our
> > INBOX. Which is why we like them to stay with the standard format.
> 
> Thanks makes sense. So Greg, I submitted this properly, does it look good to
> you now? Steven, I would appreciate any Acks/Reviews on the patch as well:
> https://lore.kernel.org/patchwork/patch/1070199/

Looks good to me, should get to it in a few days...

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Steven Rostedt]

On Tue, 7 May 2019 18:57:18 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> > Thanks makes sense. So Greg, I submitted this properly, does it look good to
> > you now? Steven, I would appreciate any Acks/Reviews on the patch as well:
> > https://lore.kernel.org/patchwork/patch/1070199/  
> 
> Looks good to me, should get to it in a few days...

Me too. Feel free to add

Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>

But as for reviewing, I see nothing wrong with it, but I doubt I'll
find something that Greg missed in this code.

-- Steve

Re: [PATCH v2] kheaders: Move from proc to sysfs

[Joel Fernandes]

On Tue, May 07, 2019 at 01:03:33PM -0400, Steven Rostedt wrote:
> On Tue, 7 May 2019 18:57:18 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > > Thanks makes sense. So Greg, I submitted this properly, does it look good to
> > > you now? Steven, I would appreciate any Acks/Reviews on the patch as well:
> > > https://lore.kernel.org/patchwork/patch/1070199/  
> > 
> > Looks good to me, should get to it in a few days...
> 
> Me too. Feel free to add
> 
> Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> 
> But as for reviewing, I see nothing wrong with it, but I doubt I'll
> find something that Greg missed in this code.

Thanks!

 - Joel