linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: catalin.marinas@arm.com (Catalin Marinas)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code
Date: Fri, 12 Sep 2014 18:21:51 +0100	[thread overview]
Message-ID: <20140912172151.GI5532@arm.com> (raw)
In-Reply-To: <54132A76.4070501@redhat.com>

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

  reply	other threads:[~2014-09-12 17:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-12  7:11 [PATCH arm64-next] net: bpf: arm64: address randomize and write protect JIT code Daniel Borkmann
2014-09-12 16:03 ` Catalin Marinas
2014-09-12 16:21   ` Daniel Borkmann
2014-09-12 16:46     ` Catalin Marinas
2014-09-12 17:10       ` Will Deacon
2014-09-12 17:16       ` Daniel Borkmann
2014-09-12 17:21         ` Catalin Marinas [this message]
2014-09-12 17:39           ` Daniel Borkmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140912172151.GI5532@arm.com \
    --to=catalin.marinas@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).