netdev - Re: [PATCH net] flow_dissector: properly cap thoff field

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d6eff065-a7aa-e1ae-fb23-70fc3bc0d8b3@redhat.com>
Date:   Thu, 18 Jan 2018 11:59:05 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Eric Dumazet <eric.dumazet@...il.com>,
        David Miller <davem@...emloft.net>
Cc:     netdev <netdev@...r.kernel.org>,
        Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] flow_dissector: properly cap thoff field



On 2018年01月18日 06:21, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@...gle.com>
>
> syzbot reported yet another crash [1] that is caused by
> insufficient validation of DODGY packets.
>
> Two bugs are happening here to trigger the crash.
>
> 1) Flow dissection leaves with incorrect thoff field.
>
> 2) skb_probe_transport_header() sets transport header to this invalid
> thoff, even if pointing after skb valid data.
>
> 3) qdisc_pkt_len_init() reads out-of-bound data because it
> trusts tcp_hdrlen(skb)
>
> Possible fixes :
>
> - Full flow dissector validation before injecting bad DODGY packets in
> the stack.
>   This approach was attempted here : https://patchwork.ozlabs.org/patch/
> 861874/
>
> - Have more robust functions in the core.
>    This might be needed anyway for stable versions.
>
> This patch fixes the flow dissection issue.
>
> [1]
> CPU: 1 PID: 3144 Comm: syzkaller271204 Not tainted 4.15.0-rc4-mm1+ #49
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:17 [inline]
>   dump_stack+0x194/0x257 lib/dump_stack.c:53
>   print_address_description+0x73/0x250 mm/kasan/report.c:256
>   kasan_report_error mm/kasan/report.c:355 [inline]
>   kasan_report+0x23b/0x360 mm/kasan/report.c:413
>   __asan_report_load2_noabort+0x14/0x20 mm/kasan/report.c:432
>   __tcp_hdrlen include/linux/tcp.h:35 [inline]
>   tcp_hdrlen include/linux/tcp.h:40 [inline]
>   qdisc_pkt_len_init net/core/dev.c:3160 [inline]
>   __dev_queue_xmit+0x20d3/0x2200 net/core/dev.c:3465
>   dev_queue_xmit+0x17/0x20 net/core/dev.c:3554
>   packet_snd net/packet/af_packet.c:2943 [inline]
>   packet_sendmsg+0x3ad5/0x60a0 net/packet/af_packet.c:2968
>   sock_sendmsg_nosec net/socket.c:628 [inline]
>   sock_sendmsg+0xca/0x110 net/socket.c:638
>   sock_write_iter+0x31a/0x5d0 net/socket.c:907
>   call_write_iter include/linux/fs.h:1776 [inline]
>   new_sync_write fs/read_write.c:469 [inline]
>   __vfs_write+0x684/0x970 fs/read_write.c:482
>   vfs_write+0x189/0x510 fs/read_write.c:544
>   SYSC_write fs/read_write.c:589 [inline]
>   SyS_write+0xef/0x220 fs/read_write.c:581
>   entry_SYSCALL_64_fastpath+0x1f/0x96
>
> Fixes: 34fad54c2537 ("net: __skb_flow_dissect() must cap its return value")
> Fixes: a6e544b0a88b ("flow_dissector: Jump to exit code in __skb_flow_dissect")
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Willem de Bruijn <willemb@...gle.com>
> Reported-by: syzbot <syzkaller@...glegroups.com>
> ---
>   net/core/flow_dissector.c |    3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 15ce300637650e17fcab7e378b20fe7972686d46..544bddf08e13c7f6e47aadc737244c9ba5af56b2 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -976,8 +976,8 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>   out_good:
>   	ret = true;
>   
> -	key_control->thoff = (u16)nhoff;
>   out:
> +	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
>   	key_basic->n_proto = proto;
>   	key_basic->ip_proto = ip_proto;
>   
> @@ -985,7 +985,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>   
>   out_bad:
>   	ret = false;
> -	key_control->thoff = min_t(u16, nhoff, skb ? skb->len : hlen);
>   	goto out;
>   }
>   EXPORT_SYMBOL(__skb_flow_dissect);
>

Acked-by: Jason Wang <jasowang@...hat.com>

I would ask for a CVE for this.

Thanks