2018-06-14 14:42:50

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 0/3] net: bpfilter: clean-up build rules


Clean-up from Kbuild/Kconfig point of view.

I confirmed this series can apply and compile
based on today's Linus tree.
(commit 2837461dbe6f)



Masahiro Yamada (3):
bpfilter: add bpfilter_umh to .gitignore
bpfilter: include bpfilter_umh in assembly instead of using objcopy
bpfilter: check compiler capability in Kconfig

Makefile | 5 -----
net/Makefile | 4 ----
net/bpfilter/.gitignore | 1 +
net/bpfilter/Kconfig | 2 +-
net/bpfilter/Makefile | 15 ++-------------
net/bpfilter/bpfilter_kern.c | 11 +++++------
net/bpfilter/bpfilter_umh_blob.S | 7 +++++++
scripts/cc-can-link.sh | 2 +-
8 files changed, 17 insertions(+), 30 deletions(-)
create mode 100644 net/bpfilter/.gitignore
create mode 100644 net/bpfilter/bpfilter_umh_blob.S

--
2.7.4



2018-06-14 14:41:43

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

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 <[email protected]>
---

Changes in v2:
- Rebase

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 e0bbe75..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 `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
- -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 0952257..6de3ae5 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_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:
--
2.7.4


2018-06-14 14:41:44

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 3/3] bpfilter: check compiler capability in Kconfig

With the brand-new syntax extension of Kconfig, we can directly
check the compiler capability in the configuration phase.

If the cc-can-link.sh fails, the BPFILTER_UMH is automatically
hidden by the dependency.

I deleted 'default n', which is no-op.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2:
- newly added

Makefile | 5 -----
net/Makefile | 4 ----
net/bpfilter/Kconfig | 2 +-
scripts/cc-can-link.sh | 2 +-
4 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/Makefile b/Makefile
index 8a26b59..9ada673 100644
--- a/Makefile
+++ b/Makefile
@@ -507,11 +507,6 @@ ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/gcc-goto.sh $(CC) $(KBUILD_CFLA
KBUILD_AFLAGS += -DCC_HAVE_ASM_GOTO
endif

-ifeq ($(shell $(CONFIG_SHELL) $(srctree)/scripts/cc-can-link.sh $(CC)), y)
- CC_CAN_LINK := y
- export CC_CAN_LINK
-endif
-
# The expansion should be delayed until arch/$(SRCARCH)/Makefile is included.
# Some architectures define CROSS_COMPILE in arch/$(SRCARCH)/Makefile.
# CC_VERSION_TEXT is referenced from Kconfig (so it needs export),
diff --git a/net/Makefile b/net/Makefile
index 13ec0d5..bdaf539 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -20,11 +20,7 @@ obj-$(CONFIG_TLS) += tls/
obj-$(CONFIG_XFRM) += xfrm/
obj-$(CONFIG_UNIX) += unix/
obj-$(CONFIG_NET) += ipv6/
-ifneq ($(CC_CAN_LINK),y)
-$(warning CC cannot link executables. Skipping bpfilter.)
-else
obj-$(CONFIG_BPFILTER) += bpfilter/
-endif
obj-$(CONFIG_PACKET) += packet/
obj-$(CONFIG_NET_KEY) += key/
obj-$(CONFIG_BRIDGE) += bridge/
diff --git a/net/bpfilter/Kconfig b/net/bpfilter/Kconfig
index a948b07..76deb66 100644
--- a/net/bpfilter/Kconfig
+++ b/net/bpfilter/Kconfig
@@ -1,6 +1,5 @@
menuconfig BPFILTER
bool "BPF based packet filtering framework (BPFILTER)"
- default n
depends on NET && BPF && INET
help
This builds experimental bpfilter framework that is aiming to
@@ -9,6 +8,7 @@ menuconfig BPFILTER
if BPFILTER
config BPFILTER_UMH
tristate "bpfilter kernel module with user mode helper"
+ depends on $(success,$(srctree)/scripts/cc-can-link.sh $(CC))
default m
help
This builds bpfilter kernel module with embedded user mode helper
diff --git a/scripts/cc-can-link.sh b/scripts/cc-can-link.sh
index 208eb28..6efcead 100755
--- a/scripts/cc-can-link.sh
+++ b/scripts/cc-can-link.sh
@@ -1,7 +1,7 @@
#!/bin/sh
# SPDX-License-Identifier: GPL-2.0

-cat << "END" | $@ -x c - -o /dev/null >/dev/null 2>&1 && echo "y"
+cat << "END" | $@ -x c - -o /dev/null >/dev/null 2>&1
#include <stdio.h>
int main(void)
{
--
2.7.4


2018-06-14 14:42:21

by Masahiro Yamada

[permalink] [raw]
Subject: [PATCH v2 1/3] bpfilter: add bpfilter_umh to .gitignore

bpfilter_umh is a generated file. It should be ignored by git.

Signed-off-by: Masahiro Yamada <[email protected]>
---

Changes in v2: None

net/bpfilter/.gitignore | 1 +
1 file changed, 1 insertion(+)
create mode 100644 net/bpfilter/.gitignore

diff --git a/net/bpfilter/.gitignore b/net/bpfilter/.gitignore
new file mode 100644
index 0000000..e97084e
--- /dev/null
+++ b/net/bpfilter/.gitignore
@@ -0,0 +1 @@
+bpfilter_umh
--
2.7.4


2018-06-15 00:49:23

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

On Thu, Jun 14, 2018 at 11:39:31PM +0900, Masahiro Yamada wrote:
> 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 <[email protected]>
> ---
>
> Changes in v2:
> - Rebase
>
> 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 e0bbe75..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 `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
> - -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 0952257..6de3ae5 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_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:

for some reason it doesn't work.
fork_usermode_blob() returns ENOEXEC
You should be able to test it simply running 'iptables -L'.
Without this patch you should see:
[ 12.696937] bpfilter: Loaded bpfilter_umh pid 225
Started bpfilter

where first line comes from kernel module and second from umh.



2018-06-26 03:46:15

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

Hi Alexei,


2018-06-15 9:47 GMT+09:00 Alexei Starovoitov <[email protected]>:
> On Thu, Jun 14, 2018 at 11:39:31PM +0900, Masahiro Yamada wrote:
>> 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 <[email protected]>
>> ---
>>
>> Changes in v2:
>> - Rebase
>>
>> 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 e0bbe75..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 `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
>> - -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 0952257..6de3ae5 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_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:
>
> for some reason it doesn't work.
> fork_usermode_blob() returns ENOEXEC
> You should be able to test it simply running 'iptables -L'.
> Without this patch you should see:
> [ 12.696937] bpfilter: Loaded bpfilter_umh pid 225
> Started bpfilter
>
> where first line comes from kernel module and second from umh.


Sorry for the late reply.

Unfortunately, I will be busy for a while.

I will come back eventually
to check it out, but I cannot tell when.


Somebody else sent a patch equivalent to 1/3, so it is fine.

3/3 can go independently, so it will send it as a separate patch for now.





--
Best Regards
Masahiro Yamada

2019-01-31 10:50:33

by Janne Karhunen

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

Hi,

Hmm, does this approach work if the code is not in a kernel module? I
tried to use it as part of the kernel image and looks to me the
bounding symbols _start and _end are not correctly relocated?

--
Janne

On Fri, Jun 15, 2018 at 3:48 AM Alexei Starovoitov
<[email protected]> wrote:
>
> On Thu, Jun 14, 2018 at 11:39:31PM +0900, Masahiro Yamada wrote:
> > 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 <[email protected]>
> > ---
> >
> > Changes in v2:
> > - Rebase
> >
> > 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 e0bbe75..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 `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
> > - -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 0952257..6de3ae5 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_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:
>
> for some reason it doesn't work.
> fork_usermode_blob() returns ENOEXEC
> You should be able to test it simply running 'iptables -L'.
> Without this patch you should see:
> [ 12.696937] bpfilter: Loaded bpfilter_umh pid 225
> Started bpfilter
>
> where first line comes from kernel module and second from umh.
>
>

2019-01-31 11:10:32

by Janne Karhunen

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

Hi,

Never mind, not enough coffee for the morning. Looks good addressing
wise, but something goes haywire with the copy. Some size limitation?

[ 84.402647] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
BIOS 1.10.2-1ubuntu1 04/01/2014
[ 84.403899] RIP: 0010:__memcpy+0x12/0x20
[ 84.404441] Code: c1 e2 20 48 09 d0 48 31 c3 e9 76 ff ff ff 90 90
90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 89 d1 48 c1 e9 03
83 e2 07 <f3> 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1
f3 a4
[ 84.407004] RSP: 0018:ffffc90000ac39c0 EFLAGS: 00010246
[ 84.407714] RAX: ffff888041709000 RBX: 0000000000001000 RCX: 0000000000000200
[ 84.408667] RDX: 0000000000000000 RSI: ffffffff82a7cb4d RDI: ffff888041709000
[ 84.409668] RBP: ffffc90000ac3a08 R08: ffff888041709000 R09: 0000000000000000
[ 84.410621] R10: 0000000000000000 R11: 000015fffefa3dbf R12: 0000000000001000
[ 84.411710] R13: 0000000000001000 R14: ffffc90000ac3b28 R15: 0000000000001000
[ 84.412675] FS: 00007f6cd3a35740(0000) GS:ffff88807da00000(0000)
knlGS:0000000000000000
[ 84.413805] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 84.414655] CR2: ffffffff82a7cb4d CR3: 0000000031344000 CR4: 00000000000006f0
[ 84.415656] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 84.416647] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 84.417619] Call Trace:
[ 84.417996] ? iov_iter_copy_from_user_atomic+0x21b/0x360
[ 84.418754] ? shmem_write_begin+0x4a/0x80
[ 84.419396] generic_perform_write+0xdd/0x1b0
[ 84.419897] __generic_file_write_iter+0x1ab/0x1d0
[ 84.420453] ? apparmor_file_alloc_security+0x4c/0x230
[ 84.421082] ? kmem_cache_alloc+0x1b5/0x1d0
[ 84.421677] generic_file_write_iter+0xb3/0x150
[ 84.422353] __vfs_write+0x145/0x1d0
[ 84.422907] vfs_write+0xae/0x1a0
[ 84.423435] kernel_write+0x55/0x70
[ 84.423996] fork_usermode_blob+0x85/0x11b
..

ret = fork_usermode_blob(&test_umh_start,
&test_umh_end - &test_umh_start, &um_info);

Addresses indeed seem ok and the '-fpic -fpie -fPIE -static' compiled
user mode blob sits in between those symbols correctly. Hummm ?


--
Janne

On Thu, Jan 31, 2019 at 12:48 PM Janne Karhunen
<[email protected]> wrote:
>
> Hi,
>
> Hmm, does this approach work if the code is not in a kernel module? I
> tried to use it as part of the kernel image and looks to me the
> bounding symbols _start and _end are not correctly relocated?
>
> --
> Janne
>
> On Fri, Jun 15, 2018 at 3:48 AM Alexei Starovoitov
> <[email protected]> wrote:
> >
> > On Thu, Jun 14, 2018 at 11:39:31PM +0900, Masahiro Yamada wrote:
> > > 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 <[email protected]>
> > > ---
> > >
> > > Changes in v2:
> > > - Rebase
> > >
> > > 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 e0bbe75..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 `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
> > > - -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 0952257..6de3ae5 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_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:
> >
> > for some reason it doesn't work.
> > fork_usermode_blob() returns ENOEXEC
> > You should be able to test it simply running 'iptables -L'.
> > Without this patch you should see:
> > [ 12.696937] bpfilter: Loaded bpfilter_umh pid 225
> > Started bpfilter
> >
> > where first line comes from kernel module and second from umh.
> >
> >

2019-01-31 12:41:46

by Janne Karhunen

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] bpfilter: include bpfilter_umh in assembly instead of using objcopy

Hi,

Okay my bad, proper __initconst declarations would do the trick, cool stuff.


--
Janne

On Thu, Jan 31, 2019 at 1:09 PM Janne Karhunen <[email protected]> wrote:
>
> Hi,
>
> Never mind, not enough coffee for the morning. Looks good addressing
> wise, but something goes haywire with the copy. Some size limitation?
>
> [ 84.402647] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS 1.10.2-1ubuntu1 04/01/2014
> [ 84.403899] RIP: 0010:__memcpy+0x12/0x20
> [ 84.404441] Code: c1 e2 20 48 09 d0 48 31 c3 e9 76 ff ff ff 90 90
> 90 90 90 90 90 90 90 90 0f 1f 44 00 00 48 89 f8 48 89 d1 48 c1 e9 03
> 83 e2 07 <f3> 48 a5 89 d1 f3 a4 c3 66 0f 1f 44 00 00 48 89 f8 48 89 d1
> f3 a4
> [ 84.407004] RSP: 0018:ffffc90000ac39c0 EFLAGS: 00010246
> [ 84.407714] RAX: ffff888041709000 RBX: 0000000000001000 RCX: 0000000000000200
> [ 84.408667] RDX: 0000000000000000 RSI: ffffffff82a7cb4d RDI: ffff888041709000
> [ 84.409668] RBP: ffffc90000ac3a08 R08: ffff888041709000 R09: 0000000000000000
> [ 84.410621] R10: 0000000000000000 R11: 000015fffefa3dbf R12: 0000000000001000
> [ 84.411710] R13: 0000000000001000 R14: ffffc90000ac3b28 R15: 0000000000001000
> [ 84.412675] FS: 00007f6cd3a35740(0000) GS:ffff88807da00000(0000)
> knlGS:0000000000000000
> [ 84.413805] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 84.414655] CR2: ffffffff82a7cb4d CR3: 0000000031344000 CR4: 00000000000006f0
> [ 84.415656] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 84.416647] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 84.417619] Call Trace:
> [ 84.417996] ? iov_iter_copy_from_user_atomic+0x21b/0x360
> [ 84.418754] ? shmem_write_begin+0x4a/0x80
> [ 84.419396] generic_perform_write+0xdd/0x1b0
> [ 84.419897] __generic_file_write_iter+0x1ab/0x1d0
> [ 84.420453] ? apparmor_file_alloc_security+0x4c/0x230
> [ 84.421082] ? kmem_cache_alloc+0x1b5/0x1d0
> [ 84.421677] generic_file_write_iter+0xb3/0x150
> [ 84.422353] __vfs_write+0x145/0x1d0
> [ 84.422907] vfs_write+0xae/0x1a0
> [ 84.423435] kernel_write+0x55/0x70
> [ 84.423996] fork_usermode_blob+0x85/0x11b
> ..
>
> ret = fork_usermode_blob(&test_umh_start,
> &test_umh_end - &test_umh_start, &um_info);
>
> Addresses indeed seem ok and the '-fpic -fpie -fPIE -static' compiled
> user mode blob sits in between those symbols correctly. Hummm ?
>
>
> --
> Janne
>
> On Thu, Jan 31, 2019 at 12:48 PM Janne Karhunen
> <[email protected]> wrote:
> >
> > Hi,
> >
> > Hmm, does this approach work if the code is not in a kernel module? I
> > tried to use it as part of the kernel image and looks to me the
> > bounding symbols _start and _end are not correctly relocated?
> >
> > --
> > Janne
> >
> > On Fri, Jun 15, 2018 at 3:48 AM Alexei Starovoitov
> > <[email protected]> wrote:
> > >
> > > On Thu, Jun 14, 2018 at 11:39:31PM +0900, Masahiro Yamada wrote:
> > > > 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 <[email protected]>
> > > > ---
> > > >
> > > > Changes in v2:
> > > > - Rebase
> > > >
> > > > 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 e0bbe75..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 `$(OBJDUMP) -f $<|grep format|cut -d' ' -f8` \
> > > > - -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 0952257..6de3ae5 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_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:
> > >
> > > for some reason it doesn't work.
> > > fork_usermode_blob() returns ENOEXEC
> > > You should be able to test it simply running 'iptables -L'.
> > > Without this patch you should see:
> > > [ 12.696937] bpfilter: Loaded bpfilter_umh pid 225
> > > Started bpfilter
> > >
> > > where first line comes from kernel module and second from umh.
> > >
> > >