Re: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code

From: Daniel Borkmann
Date: Fri Sep 12 2014 - 13:17:25 EST


On 09/12/2014 06:46 PM, Catalin Marinas wrote:
On Fri, Sep 12, 2014 at 05:21:27PM +0100, Daniel Borkmann wrote:
On 09/12/2014 06:03 PM, Catalin Marinas wrote:
On Fri, Sep 12, 2014 at 08:11:37AM +0100, Daniel Borkmann wrote:
Will, Catalin, Dave, this is more or less a heads-up: when net-next and
arm64-next tree will get both merged into Linus' tree, we will run into
a 'silent' merge conflict until someone actually runs eBPF JIT on ARM64
and might notice (I presume) an oops when JIT is freeing bpf_prog. I'd
assume nobody actually _runs_ linux-next, but not sure about that though.

Some people do.

How do we handle this? Would I need to resend this patch when the time
comes or would you ARM64 guys take care of it automagically? ;)

I think we could disable BPF for arm64 until -rc1 and re-enable it
together with this patch.

Ok, yes, that would mitigate it a bit. Sounds fine to me.

One comment below:

--- a/arch/arm64/net/bpf_jit_comp.c
+++ b/arch/arm64/net/bpf_jit_comp.c
[...]
+static void jit_fill_hole(void *area, unsigned int size)
+{
+ /* Insert illegal UND instructions. */
+ u32 *ptr, fill_ins = 0xe7ffffff;

On arm64 we don't have a guaranteed undefined instruction space (and
Will tells me that on Thumb-2 for the 32-bit arm port it actually is a
valid instruction, it seems that you used the same value).

Hm, ok, the boards we've tried out and where Zi tested it too, it worked.

So, if I try this:

$ echo 0xffffffe7 | xxd -r > test.bin
$ arm-linux-gnueabihf-objdump -m arm -D -b binary test.bin
...
0: e7ffffff udf #65535 ; 0xffff

Do you use the same constant on arm32?

I was using something along that lines, but I guess I missed
something:

# cat foo.S
.globl foobar
foobar:
.word 0xe7ffffff
# cat bar.c
#include <stdio.h>
extern void foobar(void);
int main(void)
{
foobar();
printf("!ok\n");
return 0;
}
# as foo.S -o foo.o
# gcc bar.c foo.o -o bar
# ./bar
Illegal instruction

I think the only guaranteed way is to use the BRK #imm instruction but
it requires some changes to the handling code as it is currently used
for kgdb (unless you can use two instructions for filling in which could
generate a NULL pointer access).

The trade-off would be that if we align on 8, it would certainly increase
the probability to jump to the right offset. Note, on x86_64 we have no
alignment requirements, hence 1, and on s390x only alignment of 2.

So, on that few (?) boards where UND would be a valid instruction [ as
opposed to crash the kernel ], would it translate into a NOP and just
'walk' from there into the JIT image?

On current ARMv8 CPU implementations, the above constant is unallocated
in the A64 instruction space. But you never know, it may be allocated in
the future.

I think it's easier if you just use something like BRK #0x100 (opcode
0xd4202000) which would trigger a fault in the kernel (kgdb uses #imm
0x400 and 0x401).

An unallocated BRK would trigger a fault via do_debug_exception ->
brk_handler and panic the kernel.

Okay, that's fine by me, I'll just update s/0xe7ffffff/0xd4202000/.

Do you want me to use the same suggestion for arm32 as well as it
would be less fragile?

Last but not least ;), if I would resend it today, would you queue
it for later on, or do you want to handle it differently?

Thanks again,
Daniel
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/