[PATCH v2 net] net: filter: x86: fix JIT address randomization

[PATCH v2 net] net: filter: x86: fix JIT address randomization

[Alexei Starovoitov]

bpf_alloc_binary() adds 128 bytes of room to JITed program image
and rounds it up to the nearest page size. If image size is close
to page size (like 4000), it is rounded to two pages:
round_up(4000 + 4 + 128) == 8192
then 'hole' is computed as 8192 - (4000 + 4) = 4188
If prandom_u32() % hole selects a number >= PAGE_SIZE - sizeof(*header)
then kernel will crash during bpf_jit_free():

kernel BUG at arch/x86/mm/pageattr.c:887!
Call Trace:
 [<ffffffff81037285>] change_page_attr_set_clr+0x135/0x460
 [<ffffffff81694cc0>] ? _raw_spin_unlock_irq+0x30/0x50
 [<ffffffff810378ff>] set_memory_rw+0x2f/0x40
 [<ffffffffa01a0d8d>] bpf_jit_free_deferred+0x2d/0x60
 [<ffffffff8106bf98>] process_one_work+0x1d8/0x6a0
 [<ffffffff8106bf38>] ? process_one_work+0x178/0x6a0
 [<ffffffff8106c90c>] worker_thread+0x11c/0x370

since bpf_jit_free() does:
  unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
  struct bpf_binary_header *header = (void *)addr;
to compute start address of 'bpf_binary_header'
and header->pages will pass junk to:
  set_memory_rw(addr, header->pages);

Fix it by making sure that &header->image[prandom_u32() % hole] and &header
are in the same page

Fixes: 314beb9bcabfd ("x86: bpf_jit_comp: secure bpf jit against spraying attacks")
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 arch/x86/net/bpf_jit_comp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index dc01773..6d5663a 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -171,7 +171,7 @@ static struct bpf_binary_header *bpf_alloc_binary(unsigned int proglen,
 	memset(header, 0xcc, sz); /* fill whole space with int3 instructions */
 
 	header->pages = sz / PAGE_SIZE;
-	hole = sz - (proglen + sizeof(*header));
+	hole = min(sz - (proglen + sizeof(*header)), PAGE_SIZE - sizeof(*header));
 
 	/* insert a random number of int3 instructions before BPF code */
 	*image_ptr = &header->image[prandom_u32() % hole];
-- 
1.7.9.5

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[Eric Dumazet]

On Tue, 2014-05-13 at 15:05 -0700, Alexei Starovoitov wrote:

> 
> Fix it by making sure that &header->image[prandom_u32() % hole] and &header
> are in the same page
> 
> Fixes: 314beb9bcabfd ("x86: bpf_jit_comp: secure bpf jit against spraying attacks")
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> ---
>  arch/x86/net/bpf_jit_comp.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks Alexei.

Acked-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[David Miller]

From: Alexei Starovoitov <ast@plumgrid.com>
Date: Tue, 13 May 2014 15:05:55 -0700

> bpf_alloc_binary() adds 128 bytes of room to JITed program image
> and rounds it up to the nearest page size. If image size is close
> to page size (like 4000), it is rounded to two pages:
> round_up(4000 + 4 + 128) == 8192
> then 'hole' is computed as 8192 - (4000 + 4) = 4188
> If prandom_u32() % hole selects a number >= PAGE_SIZE - sizeof(*header)
> then kernel will crash during bpf_jit_free():
> 
> kernel BUG at arch/x86/mm/pageattr.c:887!
> Call Trace:
>  [<ffffffff81037285>] change_page_attr_set_clr+0x135/0x460
>  [<ffffffff81694cc0>] ? _raw_spin_unlock_irq+0x30/0x50
>  [<ffffffff810378ff>] set_memory_rw+0x2f/0x40
>  [<ffffffffa01a0d8d>] bpf_jit_free_deferred+0x2d/0x60
>  [<ffffffff8106bf98>] process_one_work+0x1d8/0x6a0
>  [<ffffffff8106bf38>] ? process_one_work+0x178/0x6a0
>  [<ffffffff8106c90c>] worker_thread+0x11c/0x370
> 
> since bpf_jit_free() does:
>   unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
>   struct bpf_binary_header *header = (void *)addr;
> to compute start address of 'bpf_binary_header'
> and header->pages will pass junk to:
>   set_memory_rw(addr, header->pages);
> 
> Fix it by making sure that &header->image[prandom_u32() % hole] and &header
> are in the same page
> 
> Fixes: 314beb9bcabfd ("x86: bpf_jit_comp: secure bpf jit against spraying attacks")
> Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>

Applied and queued up for -stable, thank you.

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[Heiko Carstens]

On Tue, May 13, 2014 at 06:31:45PM -0400, David Miller wrote:
> From: Alexei Starovoitov <ast@plumgrid.com>
> Date: Tue, 13 May 2014 15:05:55 -0700
> 
> > Fix it by making sure that &header->image[prandom_u32() % hole] and &header
> > are in the same page
> > 
> > Fixes: 314beb9bcabfd ("x86: bpf_jit_comp: secure bpf jit against spraying attacks")
> > Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
> 
> Applied and queued up for -stable, thank you.

Ok, I didn't see that Alexei already sent a second version when asking
him to also fix s390.
Anyway, here is the s390 version of Alexei's fix which is nearly identical
to the x86 version.

Dave, can you route this via -net?

>From c4fce4d9777f1df9ca0ed503c485c3fd3db7d3ac Mon Sep 17 00:00:00 2001
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Wed, 14 May 2014 09:48:21 +0200
Subject: [PATCH] net: filter: s390: fix JIT address randomization

This is the s390 variant of Alexei's JIT bug fix.
(patch description below stolen from Alexei's patch)

bpf_alloc_binary() adds 128 bytes of room to JITed program image
and rounds it up to the nearest page size. If image size is close
to page size (like 4000), it is rounded to two pages:
round_up(4000 + 4 + 128) == 8192
then 'hole' is computed as 8192 - (4000 + 4) = 4188
If prandom_u32() % hole selects a number >= PAGE_SIZE - sizeof(*header)
then kernel will crash during bpf_jit_free():

kernel BUG at arch/x86/mm/pageattr.c:887!
Call Trace:
 [<ffffffff81037285>] change_page_attr_set_clr+0x135/0x460
 [<ffffffff81694cc0>] ? _raw_spin_unlock_irq+0x30/0x50
 [<ffffffff810378ff>] set_memory_rw+0x2f/0x40
 [<ffffffffa01a0d8d>] bpf_jit_free_deferred+0x2d/0x60
 [<ffffffff8106bf98>] process_one_work+0x1d8/0x6a0
 [<ffffffff8106bf38>] ? process_one_work+0x178/0x6a0
 [<ffffffff8106c90c>] worker_thread+0x11c/0x370

since bpf_jit_free() does:
  unsigned long addr = (unsigned long)fp->bpf_func & PAGE_MASK;
  struct bpf_binary_header *header = (void *)addr;
to compute start address of 'bpf_binary_header'
and header->pages will pass junk to:
  set_memory_rw(addr, header->pages);

Fix it by making sure that &header->image[prandom_u32() % hole] and &header
are in the same page.

Fixes: aa2d2c73c21f2 ("s390/bpf,jit: address randomize and write protect jit code")

Reported-by: Alexei Starovoitov <ast@plumgrid.com>
Cc: <stable@vger.kernel.org> # v3.11+
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
 arch/s390/net/bpf_jit_comp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 452d3ebd9d0f..e9f8fa9337fe 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -811,7 +811,7 @@ static struct bpf_binary_header *bpf_alloc_binary(unsigned int bpfsize,
 		return NULL;
 	memset(header, 0, sz);
 	header->pages = sz / PAGE_SIZE;
-	hole = sz - (bpfsize + sizeof(*header));
+	hole = min(sz - (bpfsize + sizeof(*header)), PAGE_SIZE - sizeof(*header));
 	/* Insert random number of illegal instructions before BPF code
 	 * and make sure the first instruction starts at an even address.
 	 */
-- 
1.8.5.5

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[David Miller]

From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Wed, 14 May 2014 10:05:57 +0200

> Dave, can you route this via -net?

Done.

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[H. Peter Anvin]

On 05/14/2014 01:10 PM, David Miller wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
> Date: Wed, 14 May 2014 10:05:57 +0200
> 
>> Dave, can you route this via -net?
> 

Pardon for being a bit confused, but it would seem x86 or net would be
the logical routes for this -- I don't mind at all it being routed
through net, but that would seem to be between myself (and the other x86
maintainers) and Dave...
	
	-hpa

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[Heiko Carstens]

On Wed, May 14, 2014 at 01:40:31PM -0700, H. Peter Anvin wrote:
> On 05/14/2014 01:10 PM, David Miller wrote:
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Date: Wed, 14 May 2014 10:05:57 +0200
> > 
> >> Dave, can you route this via -net?
> > 
> 
> Pardon for being a bit confused, but it would seem x86 or net would be
> the logical routes for this -- I don't mind at all it being routed
> through net, but that would seem to be between myself (and the other x86
> maintainers) and Dave...

Hi Peter,

you missed the s390 version of Alexei's patch which I sent as reply to this
thread. As usual I got a delivery failure for your zytor.com email address
(added your Intel address this time).

So the question was if Dave could add the s390 version of the patch to -net.

Thanks,
Heiko

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[Heiko Carstens]

On Wed, May 14, 2014 at 01:40:31PM -0700, H. Peter Anvin wrote:
> On 05/14/2014 01:10 PM, David Miller wrote:
> > From: Heiko Carstens <heiko.carstens@de.ibm.com>
> > Date: Wed, 14 May 2014 10:05:57 +0200
> > 
> >> Dave, can you route this via -net?
> > 
> 
> Pardon for being a bit confused, but it would seem x86 or net would be
> the logical routes for this -- I don't mind at all it being routed
> through net, but that would seem to be between myself (and the other x86
> maintainers) and Dave...

[resending - and this time really added hpa's Intel email address]

Hi Peter,

you missed the s390 version of Alexei's patch which I sent as reply to this
thread. As usual I got a delivery failure for your zytor.com email address
(added your Intel address this time).

So the question was if Dave could add the s390 version of the patch to -net.

Thanks,
Heiko

Re: [PATCH v2 net] net: filter: x86: fix JIT address randomization

[H. Peter Anvin]

On 05/14/2014 11:53 PM, Heiko Carstens wrote:
> On Wed, May 14, 2014 at 01:40:31PM -0700, H. Peter Anvin wrote:
>> On 05/14/2014 01:10 PM, David Miller wrote:
>>> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>>> Date: Wed, 14 May 2014 10:05:57 +0200
>>>
>>>> Dave, can you route this via -net?
>>>
>>
>> Pardon for being a bit confused, but it would seem x86 or net would be
>> the logical routes for this -- I don't mind at all it being routed
>> through net, but that would seem to be between myself (and the other x86
>> maintainers) and Dave...
> 
> [resending - and this time really added hpa's Intel email address]
> 
> Hi Peter,
> 
> you missed the s390 version of Alexei's patch which I sent as reply to this
> thread. As usual I got a delivery failure for your zytor.com email address
> (added your Intel address this time).
> 
> So the question was if Dave could add the s390 version of the patch to -net.
> 

OK, weird.  It seems IBM's mail server doesn't like me.  I wonder if it
is because of my using StartSSL for my TLS certificate.

	-hpa