Patchwork [v2,net-next,3/3] ARM: net: bpf_jit_32: support BPF_S_ANC_SECCOMP_LD_W instruction

login
register
mail settings
Submitter Xi Wang
Date April 27, 2013, 2:17 a.m.
Message ID <1367029047-14830-4-git-send-email-xi.wang@gmail.com>
Download mbox | patch
Permalink /patch/240074/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Xi Wang - April 27, 2013, 2:17 a.m.
This patch implements the seccomp BPF_S_ANC_SECCOMP_LD_W instruction
in ARM JIT.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
Cc: Daniel Borkmann <dborkman@redhat.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Will Drewry <wad@chromium.org>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: David Laight <david.laight@aculab.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Nicolas Schichan <nschichan@freebox.fr>
---
 arch/arm/net/bpf_jit_32.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
Daniel Borkmann - April 27, 2013, 6:27 a.m.
On 04/27/2013 04:17 AM, Xi Wang wrote:
> This patch implements the seccomp BPF_S_ANC_SECCOMP_LD_W instruction
> in ARM JIT.
>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> Cc: Daniel Borkmann <dborkman@redhat.com>
> Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
> Cc: Will Drewry <wad@chromium.org>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: David Laight <david.laight@aculab.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Nicolas Schichan <nschichan@freebox.fr>
> ---
>   arch/arm/net/bpf_jit_32.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
> index 073b085..9bfce464 100644
> --- a/arch/arm/net/bpf_jit_32.c
> +++ b/arch/arm/net/bpf_jit_32.c
> @@ -19,6 +19,7 @@
>   #include <linux/if_vlan.h>
>   #include <asm/cacheflush.h>
>   #include <asm/hwcap.h>
> +#include <asm/syscall.h>
>
>   #include "bpf_jit_32.h"
>
> @@ -845,6 +846,19 @@ b_epilogue:
>   			off = offsetof(struct sk_buff, queue_mapping);
>   			emit(ARM_LDRH_I(r_A, r_skb, off), ctx);
>   			break;
> +#ifdef CONFIG_SECCOMP_FILTER
> +		case BPF_S_ANC_SECCOMP_LD_W:
> +			if (k == offsetof(struct seccomp_data, arch)) {
> +				emit_mov_i(r_A, AUDIT_ARCH_ARM, ctx);
> +				break;
> +			}

Not an expert in ARM, but ...

Arent't you doing here a similar thing in terms of getting arch as Eric
criticized (Nicolas' implementation does not use that part btw.)? Also,
even if it would be possible here, now your 2 JIT implementations differ
in behaviour. I think this is unintended.

Besides all that, I think I also pointed you to a patch that already made
it in for ARM, not sure why you keep posting the ARM JIT implementation?

> +			ctx->seen |= SEEN_CALL;
> +			emit_mov_i(ARM_R3, (u32)seccomp_bpf_load, ctx);
> +			emit_mov_i(ARM_R0, k, ctx);
> +			emit_blx_r(ARM_R3, ctx);
> +			emit(ARM_MOV_R(r_A, ARM_R0), ctx);
> +			break;
> +#endif
>   		default:
>   			return -1;
>   		}
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Xi Wang - April 27, 2013, 6:32 p.m.
On Sat, Apr 27, 2013 at 2:27 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> Arent't you doing here a similar thing in terms of getting arch as Eric
> criticized (Nicolas' implementation does not use that part btw.)? Also,
> even if it would be possible here, now your 2 JIT implementations differ
> in behaviour. I think this is unintended.

Eric's comment was about x86, where the audit arch could change on the
fly.  For ARM, the audit arch doesn't change---syscall_get_arch()
always returns AUDIT_ARCH_ARM.

> Besides all that, I think I also pointed you to a patch that already made
> it in for ARM, not sure why you keep posting the ARM JIT implementation?

That's why I asked in the other post if you wanted me to rebase
against linux-next or net-next.  The ARM part 3/3 is not needed if
rebased against linux-next with Nicolas's patches.

- xi
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Borkmann - April 29, 2013, 10:16 a.m.
On 04/27/2013 08:32 PM, Xi Wang wrote:
> On Sat, Apr 27, 2013 at 2:27 AM, Daniel Borkmann <dborkman@redhat.com> wrote:

>> Besides all that, I think I also pointed you to a patch that already made
>> it in for ARM, not sure why you keep posting the ARM JIT implementation?
>
> That's why I asked in the other post if you wanted me to rebase
> against linux-next or net-next.  The ARM part 3/3 is not needed if
> rebased against linux-next with Nicolas's patches.

This discussion was only in terms of the unified interface, not the seccomp
JIT itself. If you speak about ``patch'' (and not ``patch set'') I assumed
you were only referring to the first one.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Schichan - April 29, 2013, 12:39 p.m.
On 04/27/2013 08:32 PM, Xi Wang wrote:
> On Sat, Apr 27, 2013 at 2:27 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
>> Arent't you doing here a similar thing in terms of getting arch as Eric
>> criticized (Nicolas' implementation does not use that part btw.)? Also,
>> even if it would be possible here, now your 2 JIT implementations differ
>> in behaviour. I think this is unintended.
>
> Eric's comment was about x86, where the audit arch could change on the
> fly.  For ARM, the audit arch doesn't change---syscall_get_arch()
> always returns AUDIT_ARCH_ARM.

Hi,

Indeed, syscall_get_arch() will only return AUDIT_ARCH_ARM on ARM right now. 
This might be more future proof to call syscall_get_arch() though. The main 
reason that comes to my mind would be an AArch64 kernel with support for 
AArch32 userland tasks. This would I expect require a different AUDIT_ARCH 
constant to differenciate between AArch64 and AArch32 tasks.

Regards,

Patch

diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 073b085..9bfce464 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -19,6 +19,7 @@ 
 #include <linux/if_vlan.h>
 #include <asm/cacheflush.h>
 #include <asm/hwcap.h>
+#include <asm/syscall.h>
 
 #include "bpf_jit_32.h"
 
@@ -845,6 +846,19 @@  b_epilogue:
 			off = offsetof(struct sk_buff, queue_mapping);
 			emit(ARM_LDRH_I(r_A, r_skb, off), ctx);
 			break;
+#ifdef CONFIG_SECCOMP_FILTER
+		case BPF_S_ANC_SECCOMP_LD_W:
+			if (k == offsetof(struct seccomp_data, arch)) {
+				emit_mov_i(r_A, AUDIT_ARCH_ARM, ctx);
+				break;
+			}
+			ctx->seen |= SEEN_CALL;
+			emit_mov_i(ARM_R3, (u32)seccomp_bpf_load, ctx);
+			emit_mov_i(ARM_R0, k, ctx);
+			emit_blx_r(ARM_R3, ctx);
+			emit(ARM_MOV_R(r_A, ARM_R0), ctx);
+			break;
+#endif
 		default:
 			return -1;
 		}