Messages in this thread | | | Date | Fri, 24 May 2013 10:23:08 -0700 | From | Jarkko Sakkinen <> | Subject | Re: [PATCH net-next] x86: bpf_jit_comp: secure bpf jit against spraying attacks |
| |
Hi Eric,
Peter talked to me about this BPF work to prevent JIT spraying attacks in the beginning of this week and I took a look at your patch.
Some comments:
* Meta-comment about patch structure: why this was a one patch and not two patches? It changes two things that are orthogonal to each other (random offset, RW -> RO change). * Should NX bit be turned on while JIT code is being prepared? * How hard it would be to read value of bpf_func pointer? If attacker is able to read that, it would compromise the whole randomization scheme. * I loved the socket creation trick in the blog post :) Are there any plans to do something about it? * How was minimum entropy of 128 bytes chose? The patch description does not explain this in anyway although it seems like decent choice.
/Jarkko
On 17.05.2013 19:37, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > hpa bringed into my attention some security related issues > with BPF JIT on x86. > > This patch makes sure the bpf generated code is marked read only, > as other kernel text sections. > > It also splits the unused space (we vmalloc() and only use a fraction of > the page) in two parts, so that the generated bpf code not starts at a > known offset in the page, but a pseudo random one. > > Refs: > http://mainisusuallyafunction.blogspot.com/2012/11/attacking-hardened-linux-systems-with.html > > Reported-by: H. Peter Anvin <hpa@zytor.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> > --- > arch/x86/net/bpf_jit_comp.c | 53 ++++++++++++++++++++++++++++++---- > 1 file changed, 47 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c > index c0212db..79c216a 100644 > --- a/arch/x86/net/bpf_jit_comp.c > +++ b/arch/x86/net/bpf_jit_comp.c > @@ -12,6 +12,7 @@ > #include <linux/netdevice.h> > #include <linux/filter.h> > #include <linux/if_vlan.h> > +#include <linux/random.h> > > /* > * Conventions : > @@ -144,6 +145,39 @@ static int pkt_type_offset(void) > return -1; > } > > +struct bpf_binary_header { > + unsigned int pages; > + /* Note : for security reasons, bpf code will follow a randomly > + * sized amount of int3 instructions > + */ > + u8 image[]; > +}; > + > +static struct bpf_binary_header *bpf_alloc_binary(unsigned int proglen, > + u8 **image_ptr) > +{ > + unsigned int sz, hole; > + struct bpf_binary_header *header; > + > + /* Most of BPF filters are really small, > + * but if some of them fill a page, allow at least > + * 128 extra bytes to insert a random section of int3 > + */ > + sz = round_up(proglen + sizeof(*header) + 128, PAGE_SIZE); > + header = module_alloc(sz); > + if (!header) > + return NULL; > + > + memset(header, 0xcc, sz); /* fill whole space with int3 instructions */ > + > + header->pages = sz / PAGE_SIZE; > + hole = sz - (proglen + sizeof(*header)); > + > + /* insert a random number of int3 instructions before BPF code */ > + *image_ptr = &header->image[prandom_u32() % hole]; > + return header; > +} > + > void bpf_jit_compile(struct sk_filter *fp) > { > u8 temp[64]; > @@ -153,6 +187,7 @@ void bpf_jit_compile(struct sk_filter *fp) > int t_offset, f_offset; > u8 t_op, f_op, seen = 0, pass; > u8 *image = NULL; > + struct bpf_binary_header *header = NULL; > u8 *func; > int pc_ret0 = -1; /* bpf index of first RET #0 instruction (if any) */ > unsigned int cleanup_addr; /* epilogue code offset */ > @@ -693,7 +728,7 @@ cond_branch: f_offset = addrs[i + filter[i].jf] - addrs[i]; > if (unlikely(proglen + ilen > oldproglen)) { > pr_err("bpb_jit_compile fatal error\n"); > kfree(addrs); > - module_free(NULL, image); > + module_free(NULL, header); > return; > } > memcpy(image + proglen, temp, ilen); > @@ -717,8 +752,8 @@ cond_branch: f_offset = addrs[i + filter[i].jf] - addrs[i]; > break; > } > if (proglen == oldproglen) { > - image = module_alloc(proglen); > - if (!image) > + header = bpf_alloc_binary(proglen, &image); > + if (!header) > goto out; > } > oldproglen = proglen; > @@ -728,7 +763,8 @@ cond_branch: f_offset = addrs[i + filter[i].jf] - addrs[i]; > bpf_jit_dump(flen, proglen, pass, image); > > if (image) { > - bpf_flush_icache(image, image + proglen); > + bpf_flush_icache(header, image + proglen); > + set_memory_ro((unsigned long)header, header->pages); > fp->bpf_func = (void *)image; > } > out: > @@ -738,6 +774,11 @@ out: > > void bpf_jit_free(struct sk_filter *fp) > { > - if (fp->bpf_func != sk_run_filter) > - module_free(NULL, fp->bpf_func); > + if (fp->bpf_func != sk_run_filter) { > + unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK; > + struct bpf_binary_header *header = (void *)addr; > + > + set_memory_rw(addr, header->pages); > + module_free(NULL, header); > + } > } > >
| |