[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 10 Oct 2018 10:22:18 -0700 (PDT)
From: David Miller <davem@...emloft.net>
To: edumazet@...gle.com
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com,
herbert@...dor.apana.org.au
Subject: Re: [PATCH net] net: make skb_partial_csum_set() more robust
against overflows
From: Eric Dumazet <edumazet@...gle.com>
Date: Wed, 10 Oct 2018 06:59:35 -0700
> syzbot managed to crash in skb_checksum_help() [1] :
>
> BUG_ON(offset + sizeof(__sum16) > skb_headlen(skb));
>
> Root cause is the following check in skb_partial_csum_set()
>
> if (unlikely(start > skb_headlen(skb)) ||
> unlikely((int)start + off > skb_headlen(skb) - 2))
> return false;
>
> If skb_headlen(skb) is 1, then (skb_headlen(skb) - 2) becomes 0xffffffff
> and the check fails to detect that ((int)start + off) is off the limit,
> since the compare is unsigned.
>
> When we fix that, then the first condition (start > skb_headlen(skb))
> becomes obsolete.
>
> Then we should also check that (skb_headroom(skb) + start) wont
> overflow 16bit field.
>
> [1]
> kernel BUG at net/core/dev.c:2880!
> invalid opcode: 0000 [#1] PREEMPT SMP KASAN
> CPU: 1 PID: 7330 Comm: syz-executor4 Not tainted 4.19.0-rc6+ #253
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> RIP: 0010:skb_checksum_help+0x9e3/0xbb0 net/core/dev.c:2880
> Code: 85 00 ff ff ff 48 c1 e8 03 42 80 3c 28 00 0f 84 09 fb ff ff 48 8b bd 00 ff ff ff e8 97 a8 b9 fb e9 f8 fa ff ff e8 2d 09 76 fb <0f> 0b 48 8b bd 28 ff ff ff e8 1f a8 b9 fb e9 b1 f6 ff ff 48 89 cf
> RSP: 0018:ffff8801d83a6f60 EFLAGS: 00010293
> RAX: ffff8801b9834380 RBX: ffff8801b9f8d8c0 RCX: ffffffff8608c6d7
> RDX: 0000000000000000 RSI: ffffffff8608cc63 RDI: 0000000000000006
> RBP: ffff8801d83a7068 R08: ffff8801b9834380 R09: 0000000000000000
> R10: ffff8801d83a76d8 R11: 0000000000000000 R12: 0000000000000001
> R13: 0000000000010001 R14: 000000000000ffff R15: 00000000000000a8
> FS: 00007f1a66db5700(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f7d77f091b0 CR3: 00000001ba252000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> skb_csum_hwoffload_help+0x8f/0xe0 net/core/dev.c:3269
> validate_xmit_skb+0xa2a/0xf30 net/core/dev.c:3312
> __dev_queue_xmit+0xc2f/0x3950 net/core/dev.c:3797
> dev_queue_xmit+0x17/0x20 net/core/dev.c:3838
> packet_snd net/packet/af_packet.c:2928 [inline]
> packet_sendmsg+0x422d/0x64c0 net/packet/af_packet.c:2953
>
> Fixes: 5ff8dda3035d ("net: Ensure partial checksum offset is inside the skb head")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Herbert Xu <herbert@...dor.apana.org.au>
> Reported-by: syzbot <syzkaller@...glegroups.com>
Applied and queued up for -stable, thanks Eric.
Powered by blists - more mailing lists