[PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[roy.qing.li]

From: Li RongQing <roy.qing.li@gmail.com>

The size of all_zeros_mac is 6 byte, but eth_hash() will access the
8 byte, and KASan reported the below bug:

[ 8596.479031] BUG: KASan: out of bounds access in __vxlan_find_mac+0x24/0x100 at addr ffffffff841514c0
[ 8596.487647] Read of size 8 by task ip/52820
[ 8596.490818] Address belongs to variable all_zeros_mac+0x0/0x40
[ 8596.496051] CPU: 0 PID: 52820 Comm: ip Tainted: G WC 4.1.15 #1
[ 8596.503520] Hardware name: HP ProLiant DL380p Gen8, BIOS P70 02/10/2014
[ 8596.509365] ffffffff841514c0 ffff88007450f0b8 ffffffff822fa5e1 0000000000000032
[ 8596.516112] ffff88007450f150 ffff88007450f138 ffffffff812dd58c ffff88007450f1d8
[ 8596.522856] ffffffff81113b80 0000000000000282 0000000000000001 ffffffff8101ee4d
[ 8596.529599] Call Trace:
[ 8596.530858] [<ffffffff822fa5e1>] dump_stack+0x4f/0x7b
[ 8596.535080] [<ffffffff812dd58c>] kasan_report_error+0x3bc/0x3f0
[ 8596.540258] [<ffffffff81113b80>] ? __lock_acquire+0x90/0x2140
[ 8596.545245] [<ffffffff8101ee4d>] ? save_stack_trace+0x2d/0x80
[ 8596.550234] [<ffffffff812dda70>] kasan_report+0x40/0x50
[ 8596.554647] [<ffffffff81b211e4>] ? __vxlan_find_mac+0x24/0x100
[ 8596.559729] [<ffffffff812dc399>] __asan_load8+0x69/0xa0
[ 8596.564141] [<ffffffff81b211e4>] __vxlan_find_mac+0x24/0x100
[ 8596.569033] [<ffffffff81b2683d>] vxlan_fdb_create+0x9d/0x570

it can be fixed by enlarging the all_zeros_mac to 8 byte, although it is
harmless; eth_hash() will be called in other place with the memory which
is larger and equal to 8 byte.

Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
---
 drivers/net/vxlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 2d88c79..b733f093 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -73,7 +73,7 @@ MODULE_PARM_DESC(log_ecn_error, "Log packets received with corrupted ECN");
 static int vxlan_net_id;
 static struct rtnl_link_ops vxlan_link_ops;
 
-static const u8 all_zeros_mac[ETH_ALEN];
+static const u8 all_zeros_mac[ETH_ALEN + 2];
 
 static int vxlan_sock_add(struct vxlan_dev *vxlan);
 
-- 
2.1.4

Re: [PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[Cong Wang]

On Thu, Jan 28, 2016 at 5:43 PM,  <roy.qing.li@gmail.com> wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> The size of all_zeros_mac is 6 byte, but eth_hash() will access the
> 8 byte, and KASan reported the below bug:


Sounds like we should fix eth_hash() (macvlan has a same function),
it should not read beyond 6 bytes.

Re: [PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[Eric Dumazet]

On Fri, 2016-01-29 at 13:41 -0800, Cong Wang wrote:
> On Thu, Jan 28, 2016 at 5:43 PM,  <roy.qing.li@gmail.com> wrote:
> > From: Li RongQing <roy.qing.li@gmail.com>
> >
> > The size of all_zeros_mac is 6 byte, but eth_hash() will access the
> > 8 byte, and KASan reported the below bug:
> 
> 
> Sounds like we should fix eth_hash() (macvlan has a same function),
> it should not read beyond 6 bytes.

Why ? We always have at least 2 bytes following ethernet address in a
packet.

Better add these 2 bytes in all_zeros_mac[] and not slow down the hash
function.

We use same tricks in eth_type_trans()

Re: [PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[Cong Wang]

On Fri, Jan 29, 2016 at 2:01 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2016-01-29 at 13:41 -0800, Cong Wang wrote:
>> On Thu, Jan 28, 2016 at 5:43 PM,  <roy.qing.li@gmail.com> wrote:
>> > From: Li RongQing <roy.qing.li@gmail.com>
>> >
>> > The size of all_zeros_mac is 6 byte, but eth_hash() will access the
>> > 8 byte, and KASan reported the below bug:
>>
>>
>> Sounds like we should fix eth_hash() (macvlan has a same function),
>> it should not read beyond 6 bytes.
>
> Why ? We always have at least 2 bytes following ethernet address in a
> packet.

Because we have to fix all the cases that don't.

>
> Better add these 2 bytes in all_zeros_mac[] and not slow down the hash
> function.
>
> We use same tricks in eth_type_trans()
>

I never doubt it works, as long as you want to audit all the cases
like this one,
sure go for it.

Re: [PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[Eric Dumazet]

On Fri, 2016-01-29 at 14:17 -0800, Cong Wang wrote:

> Because we have to fix all the cases that don't

Sure, lets do that without removing the optimizations.

macvlan control path seems to need a fix, as you pointed out.

Re: [PATCH] vxlan: fix a out of bounds access in __vxlan_find_mac

[David Miller]

From: roy.qing.li@gmail.com
Date: Fri, 29 Jan 2016 09:43:47 +0800

> From: Li RongQing <roy.qing.li@gmail.com>
> 
> The size of all_zeros_mac is 6 byte, but eth_hash() will access the
> 8 byte, and KASan reported the below bug:
 ...
> it can be fixed by enlarging the all_zeros_mac to 8 byte, although it is
> harmless; eth_hash() will be called in other place with the memory which
> is larger and equal to 8 byte.
> 
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>

Applied, thanks.