diff mbox series

bpf, powerpc: fix jit for seccomp_data access

Message ID 2f428cf9-23f3-d1f1-1524-46d6faa17386@pobox.com (mailing list archive)
State Accepted
Commit 083b20907185b076f21c265b30fe5b5f24c03d8c
Headers show
Series bpf, powerpc: fix jit for seccomp_data access | expand

Commit Message

Mark Lord Feb. 20, 2018, 7:49 p.m. UTC
I am using SECCOMP to filter syscalls on a ppc32 platform,
and noticed that the JIT compiler was failing on the BPF
even though the interpreter was working fine.

The issue was that the compiler was missing one of the instructions
used by SECCOMP, so here is a patch to enable JIT for that instruction.

Signed-Off-By:  Mark Lord <mlord@pobox.com>

Comments

Michael Ellerman Feb. 21, 2018, 12:33 a.m. UTC | #1
Mark Lord <mlord@pobox.com> writes:

> I am using SECCOMP to filter syscalls on a ppc32 platform,
> and noticed that the JIT compiler was failing on the BPF
> even though the interpreter was working fine.
>
> The issue was that the compiler was missing one of the instructions
> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>
> Signed-Off-By:  Mark Lord <mlord@pobox.com>

Thanks.

What is the failure mode without this patch? I assume when you have the
JIT enabled the seccomp filter fails to load entirely? Or do we have
logic to fall back to the interpreter?

Either way we should probably back port to stable. I assume this has
been broken since we originally added 32-bit support, so:

Fixes: eb84bab0fb38 ("ppc: Kconfig: Enable BPF JIT on ppc32")
Cc: stable@vger.kernel.org # v4.1+

cheers


> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -0500
> +++ linux/arch/powerpc/net/bpf_jit_comp.c       2018-02-20 14:41:20.805227494 -0500
> @@ -329,6 +329,9 @@ static int bpf_jit_build_body(struct bpf
>                         BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
>                         PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, len));
>                         break;
> +               case BPF_LDX | BPF_W | BPF_ABS: /* A = *((u32 *)(seccomp_data + K)); */
> +                       PPC_LWZ_OFFS(r_A, r_skb, K);
> +                       break;
>                 case BPF_LDX | BPF_W | BPF_LEN: /* X = skb->len; */
>                         PPC_LWZ_OFFS(r_X, r_skb, offsetof(struct sk_buff, len));
>                         break;
> -- 
> Mark Lord
> Real-Time Remedies Inc.
> mlord@pobox.com
Daniel Borkmann Feb. 21, 2018, 12:47 a.m. UTC | #2
On 02/21/2018 01:33 AM, Michael Ellerman wrote:
> Mark Lord <mlord@pobox.com> writes:
> 
>> I am using SECCOMP to filter syscalls on a ppc32 platform,
>> and noticed that the JIT compiler was failing on the BPF
>> even though the interpreter was working fine.
>>
>> The issue was that the compiler was missing one of the instructions
>> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>>
>> Signed-Off-By:  Mark Lord <mlord@pobox.com>
> 
> Thanks.
> 
> What is the failure mode without this patch? I assume when you have the
> JIT enabled the seccomp filter fails to load entirely? Or do we have
> logic to fall back to the interpreter?

The logic for all JITs is that once a BPF insn cannot be JITed then it falls
back to BPF interpreter. Here, since ppc32 is cBPF the path is: cBPF insn ->
cBPF JIT -> fail -> migrate cBPF to eBPF -> run in eBPF interpreter. In the
case where interpreter is compiled out (CONFIG_BPF_JIT_ALWAYS_ON), then the
filter is rejected.

> Either way we should probably back port to stable. I assume this has
> been broken since we originally added 32-bit support, so:

Note that arm32 before it was converted to be an eBPF JIT (eBPF JITs do handle
seccomp BPF in any case) was the only cBPF JIT that had it 'JIT-able', so
currently, other cBPF ones like sparc32 or mips32 don't translate it either.
Meaning, it would be nice to have as feature; I wouldn't necessarily frame
it as a bug though (or at least a stable-urgent one, imho, but I leave that
to you, of course).

> Fixes: eb84bab0fb38 ("ppc: Kconfig: Enable BPF JIT on ppc32")
> Cc: stable@vger.kernel.org # v4.1+
> 
> cheers
> 
> 
>> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -0500
>> +++ linux/arch/powerpc/net/bpf_jit_comp.c       2018-02-20 14:41:20.805227494 -0500
>> @@ -329,6 +329,9 @@ static int bpf_jit_build_body(struct bpf
>>                         BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
>>                         PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, len));
>>                         break;
>> +               case BPF_LDX | BPF_W | BPF_ABS: /* A = *((u32 *)(seccomp_data + K)); */
>> +                       PPC_LWZ_OFFS(r_A, r_skb, K);
>> +                       break;
>>                 case BPF_LDX | BPF_W | BPF_LEN: /* X = skb->len; */
>>                         PPC_LWZ_OFFS(r_X, r_skb, offsetof(struct sk_buff, len));
>>                         break;
>> -- 
>> Mark Lord
>> Real-Time Remedies Inc.
>> mlord@pobox.com
Michael Ellerman Feb. 22, 2018, 3:34 a.m. UTC | #3
Daniel Borkmann <daniel@iogearbox.net> writes:

> On 02/21/2018 01:33 AM, Michael Ellerman wrote:
>> Mark Lord <mlord@pobox.com> writes:
>> 
>>> I am using SECCOMP to filter syscalls on a ppc32 platform,
>>> and noticed that the JIT compiler was failing on the BPF
>>> even though the interpreter was working fine.
>>>
>>> The issue was that the compiler was missing one of the instructions
>>> used by SECCOMP, so here is a patch to enable JIT for that instruction.
>>>
>>> Signed-Off-By:  Mark Lord <mlord@pobox.com>
>> 
>> Thanks.
>> 
>> What is the failure mode without this patch? I assume when you have the
>> JIT enabled the seccomp filter fails to load entirely? Or do we have
>> logic to fall back to the interpreter?
>
> The logic for all JITs is that once a BPF insn cannot be JITed then it falls
> back to BPF interpreter. Here, since ppc32 is cBPF the path is: cBPF insn ->
> cBPF JIT -> fail -> migrate cBPF to eBPF -> run in eBPF interpreter. In the
> case where interpreter is compiled out (CONFIG_BPF_JIT_ALWAYS_ON), then the
> filter is rejected.

OK thanks.

>> Either way we should probably back port to stable. I assume this has
>> been broken since we originally added 32-bit support, so:
>
> Note that arm32 before it was converted to be an eBPF JIT (eBPF JITs do handle
> seccomp BPF in any case) was the only cBPF JIT that had it 'JIT-able', so
> currently, other cBPF ones like sparc32 or mips32 don't translate it either.
> Meaning, it would be nice to have as feature; I wouldn't necessarily frame
> it as a bug though (or at least a stable-urgent one, imho, but I leave that
> to you, of course).

OK, I'll just add the Fixes tag for our reference and we can always back
port it to stable later if we want to.

cheers
Michael Ellerman Feb. 23, 2018, 5:39 a.m. UTC | #4
On Tue, 2018-02-20 at 19:49:20 UTC, Mark Lord wrote:
> I am using SECCOMP to filter syscalls on a ppc32 platform,
> and noticed that the JIT compiler was failing on the BPF
> even though the interpreter was working fine.
> 
> The issue was that the compiler was missing one of the instructions
> used by SECCOMP, so here is a patch to enable JIT for that instruction.
> 
> Signed-Off-By:  Mark Lord <mlord@pobox.com>
> 
> --- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -0500
> +++ linux/arch/powerpc/net/bpf_jit_comp.c       2018-02-20 14:41:20.805227494 -0500
> @@ -329,6 +329,9 @@ static int bpf_jit_build_body(struct bpf
>                         BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
>                         PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, len));
>                         break;
> +               case BPF_LDX | BPF_W | BPF_ABS: /* A = *((u32 *)(seccomp_data + K)); */
> +                       PPC_LWZ_OFFS(r_A, r_skb, K);
> +                       break;
>                 case BPF_LDX | BPF_W | BPF_LEN: /* X = skb->len; */
>                         PPC_LWZ_OFFS(r_X, r_skb, offsetof(struct sk_buff, len));
>                         break;

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/083b20907185b076f21c265b30fe5b

cheers
diff mbox series

Patch

--- old/arch/powerpc/net/bpf_jit_comp.c 2018-02-16 14:07:01.000000000 -0500
+++ linux/arch/powerpc/net/bpf_jit_comp.c       2018-02-20 14:41:20.805227494 -0500
@@ -329,6 +329,9 @@  static int bpf_jit_build_body(struct bpf
                        BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, len) != 4);
                        PPC_LWZ_OFFS(r_A, r_skb, offsetof(struct sk_buff, len));
                        break;
+               case BPF_LDX | BPF_W | BPF_ABS: /* A = *((u32 *)(seccomp_data + K)); */
+                       PPC_LWZ_OFFS(r_A, r_skb, K);
+                       break;
                case BPF_LDX | BPF_W | BPF_LEN: /* X = skb->len; */
                        PPC_LWZ_OFFS(r_X, r_skb, offsetof(struct sk_buff, len));
                        break;