* Re: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
@ 2014-09-12 16:46 ` Catalin Marinas
0 siblings, 0 replies; 16+ messages in thread
From: Catalin Marinas @ 2014-09-12 16:46 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Will Deacon, davem@davemloft.net, zlim.lnx@gmail.com,
ast@plumgrid.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
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 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.
--
Catalin
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
2014-09-12 16:46 ` Catalin Marinas
@ 2014-09-12 17:10 ` Will Deacon
-1 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2014-09-12 17:10 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 12, 2014 at 05:46:57PM +0100, 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
...and for Thumb, it ends up as:
0: ffff e7ff vqshl.u64 q15, <illegal reg q15.5>, #63
which does happen to be undefined, but it feels fragile to rely on that
particular instruction form always having UNDEF behaviour.
> Do you use the same constant on arm32?
>
> > > 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.
Sounds sensible.
Will
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
@ 2014-09-12 17:10 ` Will Deacon
0 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2014-09-12 17:10 UTC (permalink / raw)
To: Catalin Marinas
Cc: Daniel Borkmann, davem@davemloft.net, zlim.lnx@gmail.com,
ast@plumgrid.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On Fri, Sep 12, 2014 at 05:46:57PM +0100, 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
...and for Thumb, it ends up as:
0: ffff e7ff vqshl.u64 q15, <illegal reg q15.5>, #63
which does happen to be undefined, but it feels fragile to rely on that
particular instruction form always having UNDEF behaviour.
> Do you use the same constant on arm32?
>
> > > 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.
Sounds sensible.
Will
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
2014-09-12 16:46 ` Catalin Marinas
@ 2014-09-12 17:16 ` Daniel Borkmann
-1 siblings, 0 replies; 16+ messages in thread
From: Daniel Borkmann @ 2014-09-12 17:16 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
@ 2014-09-12 17:16 ` Daniel Borkmann
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Borkmann @ 2014-09-12 17:16 UTC (permalink / raw)
To: Catalin Marinas
Cc: Will Deacon, davem@davemloft.net, zlim.lnx@gmail.com,
ast@plumgrid.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
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
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
2014-09-12 17:16 ` Daniel Borkmann
@ 2014-09-12 17:21 ` Catalin Marinas
-1 siblings, 0 replies; 16+ messages in thread
From: Catalin Marinas @ 2014-09-12 17:21 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Sep 12, 2014 at 06:16:38PM +0100, Daniel Borkmann wrote:
> 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:
> >>>> --- 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
That's missing a mov pc, lr.
> # cat bar.c
> #include <stdio.h>
> extern void foobar(void);
> int main(void)
> {
> foobar();
So you call it here and ends up in some data section, possibly hitting
some undefined instruction. For ARM it is undefined, for Thumb-2 it is
not as Will pointed out.
> >>> 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?
We don't have a brk instruction for arm32 but we have guaranteed
undefined space. Have a look at the kgdb support for example (or grep
for register_undef_hook under arch/arm) to get an idea.
> 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?
You can send it now, it will be pushed upstream at the right time.
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
@ 2014-09-12 17:21 ` Catalin Marinas
0 siblings, 0 replies; 16+ messages in thread
From: Catalin Marinas @ 2014-09-12 17:21 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Will Deacon, davem@davemloft.net, zlim.lnx@gmail.com,
ast@plumgrid.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On Fri, Sep 12, 2014 at 06:16:38PM +0100, Daniel Borkmann wrote:
> 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:
> >>>> --- 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
That's missing a mov pc, lr.
> # cat bar.c
> #include <stdio.h>
> extern void foobar(void);
> int main(void)
> {
> foobar();
So you call it here and ends up in some data section, possibly hitting
some undefined instruction. For ARM it is undefined, for Thumb-2 it is
not as Will pointed out.
> >>> 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?
We don't have a brk instruction for arm32 but we have guaranteed
undefined space. Have a look at the kgdb support for example (or grep
for register_undef_hook under arch/arm) to get an idea.
> 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?
You can send it now, it will be pushed upstream at the right time.
Thanks.
--
Catalin
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
2014-09-12 17:21 ` Catalin Marinas
@ 2014-09-12 17:39 ` Daniel Borkmann
-1 siblings, 0 replies; 16+ messages in thread
From: Daniel Borkmann @ 2014-09-12 17:39 UTC (permalink / raw)
To: linux-arm-kernel
On 09/12/2014 07:21 PM, Catalin Marinas wrote:
...
> We don't have a brk instruction for arm32 but we have guaranteed
> undefined space. Have a look at the kgdb support for example (or grep
> for register_undef_hook under arch/arm) to get an idea.
Will do, thanks!
>> 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?
>
> You can send it now, it will be pushed upstream at the right time.
Just did, thanks a lot for your help!
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
@ 2014-09-12 17:39 ` Daniel Borkmann
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Borkmann @ 2014-09-12 17:39 UTC (permalink / raw)
To: Catalin Marinas
Cc: Will Deacon, davem@davemloft.net, zlim.lnx@gmail.com,
ast@plumgrid.com, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
On 09/12/2014 07:21 PM, Catalin Marinas wrote:
...
> We don't have a brk instruction for arm32 but we have guaranteed
> undefined space. Have a look at the kgdb support for example (or grep
> for register_undef_hook under arch/arm) to get an idea.
Will do, thanks!
>> 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?
>
> You can send it now, it will be pushed upstream at the right time.
Just did, thanks a lot for your help!
^ permalink raw reply [flat|nested] 16+ messages in thread