[net] bpfilter: include bpfilter_umh in assembly instead of using objcopy

Message ID 20180627031348.285964-1-ast@kernel.org
State New
Headers show
Series
  • [net] bpfilter: include bpfilter_umh in assembly instead of using objcopy
Related show

Commit Message

Alexei Starovoitov June 27, 2018, 3:13 a.m.
From: Masahiro Yamada <yamada.masahiro@socionext.com>


What we want here is to embed a user-space program into the kernel.
Instead of the complex ELF magic, let's simply wrap it in the assembly
with the '.incbin' directive.

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

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

---
I think this patch should 'fix' bpfilter build issue on all archs.
cflags for cross CC may still be incorrect and embedded blob
may fail to execute via fork_usermode_blob()
(like in case of 'make ARCH=i386 net/bpfilter/' CC will build and link 64-bit
binary that will be included into bpfilter.o or vmlinux and that binary
will fail to run on 32-bit kernel),
but that is separate issue that will be addressed in net-next time frame.
Long term we've discussed to switch to something like klibc and keep it
as part of the kernel to avoid relying on glibc and cc-can-link.sh.

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

-- 
2.17.1

Comments

Guenter Roeck June 27, 2018, 4:18 p.m. | #1
On Tue, Jun 26, 2018 at 08:13:48PM -0700, Alexei Starovoitov wrote:
> From: Masahiro Yamada <yamada.masahiro@socionext.com>

> 

> What we want here is to embed a user-space program into the kernel.

> Instead of the complex ELF magic, let's simply wrap it in the assembly

> with the '.incbin' directive.

> 

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

> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

> ---

> I think this patch should 'fix' bpfilter build issue on all archs.

> cflags for cross CC may still be incorrect and embedded blob

> may fail to execute via fork_usermode_blob()

> (like in case of 'make ARCH=i386 net/bpfilter/' CC will build and link 64-bit

> binary that will be included into bpfilter.o or vmlinux and that binary

> will fail to run on 32-bit kernel),

> but that is separate issue that will be addressed in net-next time frame.

> Long term we've discussed to switch to something like klibc and keep it

> as part of the kernel to avoid relying on glibc and cc-can-link.sh.

> 


At the very least it fixes the build issue when building i386 images
on x86_64 systems.

(Build-)Tested-by: Guenter Roeck <linux@roeck-us.net>

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

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

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

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

>  create mode 100644 net/bpfilter/bpfilter_umh_blob.S

> 

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

> index 051dc18b8ccb..39c6980b5d99 100644

> --- a/net/bpfilter/Makefile

> +++ b/net/bpfilter/Makefile

> @@ -15,20 +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 \

> -          `LC_ALL=C $(OBJDUMP) -f net/bpfilter/bpfilter_umh \

> -          |awk -F' |,' '/file format/{print "-O",$$NF} \

> -          /^architecture:/{print "-B",$$2}'` \

> -      --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 09522573f611..f0fc182d3db7 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 */

> @@ -93,7 +90,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_start,

> +				 &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 000000000000..40311d10d2f2

> --- /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:
David Miller June 28, 2018, 12:43 p.m. | #2
From: Alexei Starovoitov <ast@kernel.org>

Date: Tue, 26 Jun 2018 20:13:48 -0700

> From: Masahiro Yamada <yamada.masahiro@socionext.com>

> 

> What we want here is to embed a user-space program into the kernel.

> Instead of the complex ELF magic, let's simply wrap it in the assembly

> with the '.incbin' directive.

> 

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

> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


Applied.

Patch

diff --git a/net/bpfilter/Makefile b/net/bpfilter/Makefile
index 051dc18b8ccb..39c6980b5d99 100644
--- a/net/bpfilter/Makefile
+++ b/net/bpfilter/Makefile
@@ -15,20 +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 \
-          `LC_ALL=C $(OBJDUMP) -f net/bpfilter/bpfilter_umh \
-          |awk -F' |,' '/file format/{print "-O",$$NF} \
-          /^architecture:/{print "-B",$$2}'` \
-      --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 09522573f611..f0fc182d3db7 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 */
@@ -93,7 +90,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_start,
+				 &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 000000000000..40311d10d2f2
--- /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: