diff mbox series

[2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

Message ID 1528477930-7342-3-git-send-email-yamada.masahiro@socionext.com
State Superseded
Headers show
Series net: bpfilter: clean-up build rules | expand

Commit Message

Masahiro Yamada June 8, 2018, 5:12 p.m. UTC 
Do not use the troublesome ELF magic.  What is happening here is to
embed a user-space program into the kernel.  Simply wrap it in the
assembly with the '.incbin' directive.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

---

 net/bpfilter/Makefile            | 15 ++-------------
 net/bpfilter/bpfilter_kern.c     | 11 +++++------
 net/bpfilter/bpfilter_umh_blob.S |  7 +++++++
 3 files changed, 14 insertions(+), 19 deletions(-)
 create mode 100644 net/bpfilter/bpfilter_umh_blob.S

-- 
2.7.4

Comments

Alexei Starovoitov June 8, 2018, 8:47 p.m. UTC | #1 
On Sat, Jun 09, 2018 at 02:12:09AM +0900, Masahiro Yamada wrote:
> Do not use the troublesome ELF magic.  What is happening here is to

> embed a user-space program into the kernel.  Simply wrap it in the

> assembly with the '.incbin' directive.

> 

> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

> ---

> 

>  net/bpfilter/Makefile            | 15 ++-------------

>  net/bpfilter/bpfilter_kern.c     | 11 +++++------

>  net/bpfilter/bpfilter_umh_blob.S |  7 +++++++

>  3 files changed, 14 insertions(+), 19 deletions(-)

>  create mode 100644 net/bpfilter/bpfilter_umh_blob.S

> 

> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile

> index aafa720..39c6980 100644

> --- a/net/bpfilter/Makefile

> +++ b/net/bpfilter/Makefile

> @@ -15,18 +15,7 @@ ifeq ($(CONFIG_BPFILTER_UMH), y)

>  HOSTLDFLAGS += -static

>  endif

>  

> -# a bit of elf magic to convert bpfilter_umh binary into a binary blob

> -# inside bpfilter_umh.o elf file referenced by

> -# _binary_net_bpfilter_bpfilter_umh_start symbol

> -# which bpfilter_kern.c passes further into umh blob loader at run-time

> -quiet_cmd_copy_umh = GEN $@

> -      cmd_copy_umh = echo ':' > $(obj)/.bpfilter_umh.o.cmd; \

> -      $(OBJCOPY) -I binary -O $(CONFIG_OUTPUT_FORMAT) \

> -      -B `$(OBJDUMP) -f $<|grep architecture|cut -d, -f1|cut -d' ' -f2` \

> -      --rename-section .data=.init.rodata $< $@

> -

> -$(obj)/bpfilter_umh.o: $(obj)/bpfilter_umh

> -	$(call cmd,copy_umh)

> +$(obj)/bpfilter_umh_blob.o: $(obj)/bpfilter_umh

>  

>  obj-$(CONFIG_BPFILTER_UMH) += bpfilter.o

> -bpfilter-objs += bpfilter_kern.o bpfilter_umh.o

> +bpfilter-objs += bpfilter_kern.o bpfilter_umh_blob.o

> diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c

> index b13d058..fcc1a7c 100644

> --- a/net/bpfilter/bpfilter_kern.c

> +++ b/net/bpfilter/bpfilter_kern.c

> @@ -10,11 +10,8 @@

>  #include <linux/file.h>

>  #include "msgfmt.h"

>  

> -#define UMH_start _binary_net_bpfilter_bpfilter_umh_start

> -#define UMH_end _binary_net_bpfilter_bpfilter_umh_end

> -

> -extern char UMH_start;

> -extern char UMH_end;

> +extern char bpfilter_umh_start;

> +extern char bpfilter_umh_end;

>  

>  static struct umh_info info;

>  /* since ip_getsockopt() can run in parallel, serialize access to umh */

> @@ -89,7 +86,9 @@ static int __init load_umh(void)

>  	int err;

>  

>  	/* fork usermode process */

> -	err = fork_usermode_blob(&UMH_start, &UMH_end - &UMH_start, &info);

> +	err = fork_usermode_blob(&bpfilter_umh_end,

> +				 &bpfilter_umh_end - &bpfilter_umh_start,

> +				 &info);

>  	if (err)

>  		return err;

>  	pr_info("Loaded bpfilter_umh pid %d\n", info.pid);

> diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S

> new file mode 100644

> index 0000000..40311d1

> --- /dev/null

> +++ b/net/bpfilter/bpfilter_umh_blob.S

> @@ -0,0 +1,7 @@

> +/* SPDX-License-Identifier: GPL-2.0 */

> +	.section .init.rodata, "a"

> +	.global bpfilter_umh_start

> +bpfilter_umh_start:

> +	.incbin "net/bpfilter/bpfilter_umh"


Interesting. I think this is good idea. Looks cleaner than objcopy magic.
btw CONFIG_OUTPUT_FORMAT already fixed by
commit 8d97ca6b6755 ("bpfilter: fix OUTPUT_FORMAT") in net tree.
Could you please rebase on top of that tree?
Masahiro Yamada June 9, 2018, 12:45 p.m. UTC | #2 
2018-06-09 5:47 GMT+09:00 Alexei Starovoitov <alexei.starovoitov@gmail.com>:
> On Sat, Jun 09, 2018 at 02:12:09AM +0900, Masahiro Yamada wrote:

>> Do not use the troublesome ELF magic.  What is happening here is to

>> embed a user-space program into the kernel.  Simply wrap it in the

>> assembly with the '.incbin' directive.

>>

>> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>

>> ---

>>

>>  net/bpfilter/Makefile            | 15 ++-------------

>>  net/bpfilter/bpfilter_kern.c     | 11 +++++------

>>  net/bpfilter/bpfilter_umh_blob.S |  7 +++++++

>>  3 files changed, 14 insertions(+), 19 deletions(-)

>>  create mode 100644 net/bpfilter/bpfilter_umh_blob.S

>>

>> diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile

>> index aafa720..39c6980 100644

>> --- a/net/bpfilter/Makefile

>> +++ b/net/bpfilter/Makefile

>> @@ -15,18 +15,7 @@ ifeq ($(CONFIG_BPFILTER_UMH), y)

>>  HOSTLDFLAGS += -static

>>  endif

>>

>> -# a bit of elf magic to convert bpfilter_umh binary into a binary blob

>> -# inside bpfilter_umh.o elf file referenced by

>> -# _binary_net_bpfilter_bpfilter_umh_start symbol

>> -# which bpfilter_kern.c passes further into umh blob loader at run-time

>> -quiet_cmd_copy_umh = GEN $@

>> -      cmd_copy_umh = echo ':' > $(obj)/.bpfilter_umh.o.cmd; \

>> -      $(OBJCOPY) -I binary -O $(CONFIG_OUTPUT_FORMAT) \

>> -      -B `$(OBJDUMP) -f $<|grep architecture|cut -d, -f1|cut -d' ' -f2` \

>> -      --rename-section .data=.init.rodata $< $@

>> -

>> -$(obj)/bpfilter_umh.o: $(obj)/bpfilter_umh

>> -     $(call cmd,copy_umh)

>> +$(obj)/bpfilter_umh_blob.o: $(obj)/bpfilter_umh

>>

>>  obj-$(CONFIG_BPFILTER_UMH) += bpfilter.o

>> -bpfilter-objs += bpfilter_kern.o bpfilter_umh.o

>> +bpfilter-objs += bpfilter_kern.o bpfilter_umh_blob.o

>> diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c

>> index b13d058..fcc1a7c 100644

>> --- a/net/bpfilter/bpfilter_kern.c

>> +++ b/net/bpfilter/bpfilter_kern.c

>> @@ -10,11 +10,8 @@

>>  #include <linux/file.h>

>>  #include "msgfmt.h"

>>

>> -#define UMH_start _binary_net_bpfilter_bpfilter_umh_start

>> -#define UMH_end _binary_net_bpfilter_bpfilter_umh_end

>> -

>> -extern char UMH_start;

>> -extern char UMH_end;

>> +extern char bpfilter_umh_start;

>> +extern char bpfilter_umh_end;

>>

>>  static struct umh_info info;

>>  /* since ip_getsockopt() can run in parallel, serialize access to umh */

>> @@ -89,7 +86,9 @@ static int __init load_umh(void)

>>       int err;

>>

>>       /* fork usermode process */

>> -     err = fork_usermode_blob(&UMH_start, &UMH_end - &UMH_start, &info);

>> +     err = fork_usermode_blob(&bpfilter_umh_end,

>> +                              &bpfilter_umh_end - &bpfilter_umh_start,

>> +                              &info);

>>       if (err)

>>               return err;

>>       pr_info("Loaded bpfilter_umh pid %d\n", info.pid);

>> diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S

>> new file mode 100644

>> index 0000000..40311d1

>> --- /dev/null

>> +++ b/net/bpfilter/bpfilter_umh_blob.S

>> @@ -0,0 +1,7 @@

>> +/* SPDX-License-Identifier: GPL-2.0 */

>> +     .section .init.rodata, "a"

>> +     .global bpfilter_umh_start

>> +bpfilter_umh_start:

>> +     .incbin "net/bpfilter/bpfilter_umh"

>

> Interesting. I think this is good idea. Looks cleaner than objcopy magic.

> btw CONFIG_OUTPUT_FORMAT already fixed by

> commit 8d97ca6b6755 ("bpfilter: fix OUTPUT_FORMAT") in net tree.

> Could you please rebase on top of that tree?

>



OK, I will rebase it.

BTW, I only compile-tested this patch.
Could you check if it really works?



-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
index aafa720..39c6980 100644
--- a/net/bpfilter/Makefile
+++ b/net/bpfilter/Makefile
@@ -15,18 +15,7 @@  ifeq ($(CONFIG_BPFILTER_UMH), y)
 HOSTLDFLAGS += -static
 endif
 
-# a bit of elf magic to convert bpfilter_umh binary into a binary blob
-# inside bpfilter_umh.o elf file referenced by
-# _binary_net_bpfilter_bpfilter_umh_start symbol
-# which bpfilter_kern.c passes further into umh blob loader at run-time
-quiet_cmd_copy_umh = GEN $@
-      cmd_copy_umh = echo ':' > $(obj)/.bpfilter_umh.o.cmd; \
-      $(OBJCOPY) -I binary -O $(CONFIG_OUTPUT_FORMAT) \
-      -B `$(OBJDUMP) -f $<|grep architecture|cut -d, -f1|cut -d' ' -f2` \
-      --rename-section .data=.init.rodata $< $@
-
-$(obj)/bpfilter_umh.o: $(obj)/bpfilter_umh
-	$(call cmd,copy_umh)
+$(obj)/bpfilter_umh_blob.o: $(obj)/bpfilter_umh
 
 obj-$(CONFIG_BPFILTER_UMH) += bpfilter.o
-bpfilter-objs += bpfilter_kern.o bpfilter_umh.o
+bpfilter-objs += bpfilter_kern.o bpfilter_umh_blob.o
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index b13d058..fcc1a7c 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -10,11 +10,8 @@ 
 #include <linux/file.h>
 #include "msgfmt.h"
 
-#define UMH_start _binary_net_bpfilter_bpfilter_umh_start
-#define UMH_end _binary_net_bpfilter_bpfilter_umh_end
-
-extern char UMH_start;
-extern char UMH_end;
+extern char bpfilter_umh_start;
+extern char bpfilter_umh_end;
 
 static struct umh_info info;
 /* since ip_getsockopt() can run in parallel, serialize access to umh */
@@ -89,7 +86,9 @@  static int __init load_umh(void)
 	int err;
 
 	/* fork usermode process */
-	err = fork_usermode_blob(&UMH_start, &UMH_end - &UMH_start, &info);
+	err = fork_usermode_blob(&bpfilter_umh_end,
+				 &bpfilter_umh_end - &bpfilter_umh_start,
+				 &info);
 	if (err)
 		return err;
 	pr_info("Loaded bpfilter_umh pid %d\n", info.pid);
diff --git a/net/bpfilter/bpfilter_umh_blob.S b/net/bpfilter/bpfilter_umh_blob.S
new file mode 100644
index 0000000..40311d1
--- /dev/null
+++ b/net/bpfilter/bpfilter_umh_blob.S
@@ -0,0 +1,7 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+	.section .init.rodata, "a"
+	.global bpfilter_umh_start
+bpfilter_umh_start:
+	.incbin "net/bpfilter/bpfilter_umh"
+	.global bpfilter_umh_end
+bpfilter_umh_end: