netdev - Re: [PATCH v2 net] net/tcp: fix possible out-of-bounds reads in tcp_hash

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <5424326f-19ef-4234-8723-7f16f9d7036a@arista.com>
Date: Wed, 1 Nov 2023 17:12:23 +0000
From: Dmitry Safonov <dima@...sta.com>
To: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com,
 syzbot <syzkaller@...glegroups.com>, Francesco Ruggeri
 <fruggeri@...sta.com>, David Ahern <dsahern@...nel.org>
Subject: Re: [PATCH v2 net] net/tcp: fix possible out-of-bounds reads in
 tcp_hash_fail()

On 11/1/23 04:52, Eric Dumazet wrote:
> syzbot managed to trigger a fault by sending TCP packets
> with all flags being set.
> 
> v2:
>  - While fixing this bug, add PSH flag handling and represent
>    flags the way tcpdump does : [S], [S.], [P.]
>  - Print 4-tuples more consistently between families.
> 
> BUG: KASAN: stack-out-of-bounds in string_nocheck lib/vsprintf.c:645 [inline]
> BUG: KASAN: stack-out-of-bounds in string+0x394/0x3d0 lib/vsprintf.c:727
> Read of size 1 at addr ffffc9000397f3f5 by task syz-executor299/5039
> 
> CPU: 1 PID: 5039 Comm: syz-executor299 Not tainted 6.6.0-rc7-syzkaller-02075-g55c900477f5b #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/09/2023
> Call Trace:
> <TASK>
> __dump_stack lib/dump_stack.c:88 [inline]
> dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
> print_address_description mm/kasan/report.c:364 [inline]
> print_report+0xc4/0x620 mm/kasan/report.c:475
> kasan_report+0xda/0x110 mm/kasan/report.c:588
> string_nocheck lib/vsprintf.c:645 [inline]
> string+0x394/0x3d0 lib/vsprintf.c:727
> vsnprintf+0xc5f/0x1870 lib/vsprintf.c:2818
> vprintk_store+0x3a0/0xb80 kernel/printk/printk.c:2191
> vprintk_emit+0x14c/0x5f0 kernel/printk/printk.c:2288
> vprintk+0x7b/0x90 kernel/printk/printk_safe.c:45
> _printk+0xc8/0x100 kernel/printk/printk.c:2332
> tcp_inbound_hash.constprop.0+0xdb2/0x10d0 include/net/tcp.h:2760
> tcp_v6_rcv+0x2b31/0x34d0 net/ipv6/tcp_ipv6.c:1882
> ip6_protocol_deliver_rcu+0x33b/0x13d0 net/ipv6/ip6_input.c:438
> ip6_input_finish+0x14f/0x2f0 net/ipv6/ip6_input.c:483
> NF_HOOK include/linux/netfilter.h:314 [inline]
> NF_HOOK include/linux/netfilter.h:308 [inline]
> ip6_input+0xce/0x440 net/ipv6/ip6_input.c:492
> dst_input include/net/dst.h:461 [inline]
> ip6_rcv_finish net/ipv6/ip6_input.c:79 [inline]
> NF_HOOK include/linux/netfilter.h:314 [inline]
> NF_HOOK include/linux/netfilter.h:308 [inline]
> ipv6_rcv+0x563/0x720 net/ipv6/ip6_input.c:310
> __netif_receive_skb_one_core+0x115/0x180 net/core/dev.c:5527
> __netif_receive_skb+0x1f/0x1b0 net/core/dev.c:5641
> netif_receive_skb_internal net/core/dev.c:5727 [inline]
> netif_receive_skb+0x133/0x700 net/core/dev.c:5786
> tun_rx_batched+0x429/0x780 drivers/net/tun.c:1579
> tun_get_user+0x29e7/0x3bc0 drivers/net/tun.c:2002
> tun_chr_write_iter+0xe8/0x210 drivers/net/tun.c:2048
> call_write_iter include/linux/fs.h:1956 [inline]
> new_sync_write fs/read_write.c:491 [inline]
> vfs_write+0x650/0xe40 fs/read_write.c:584
> ksys_write+0x12f/0x250 fs/read_write.c:637
> do_syscall_x64 arch/x86/entry/common.c:50 [inline]
> do_syscall_64+0x38/0xb0 arch/x86/entry/common.c:80
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> 
> Fixes: 2717b5adea9e ("net/tcp: Add tcp_hash_fail() ratelimited logs")
> Reported-by: syzbot <syzkaller@...glegroups.com>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Dmitry Safonov <dima@...sta.com>
> Cc: Francesco Ruggeri <fruggeri@...sta.com>
> Cc: David Ahern <dsahern@...nel.org>

LGTM, thanks again!

Reviewed-by: Dmitry Safonov <dima@...sta.com>

> ---
>  include/net/tcp_ao.h | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
> index a375a171ef3cb37ab1d8246c72c6a3e83f5c9184..b56be10838f09a2cb56ab511242d2b583eb4c33b 100644
> --- a/include/net/tcp_ao.h
> +++ b/include/net/tcp_ao.h
> @@ -124,7 +124,7 @@ struct tcp_ao_info {
>  #define tcp_hash_fail(msg, family, skb, fmt, ...)			\
>  do {									\
>  	const struct tcphdr *th = tcp_hdr(skb);				\
> -	char hdr_flags[5] = {};						\
> +	char hdr_flags[6];						\
>  	char *f = hdr_flags;						\
>  									\
>  	if (th->fin)							\
> @@ -133,17 +133,18 @@ do {									\
>  		*f++ = 'S';						\
>  	if (th->rst)							\
>  		*f++ = 'R';						\
> +	if (th->psh)							\
> +		*f++ = 'P';						\
>  	if (th->ack)							\
> -		*f++ = 'A';						\
> -	if (f != hdr_flags)						\
> -		*f = ' ';						\
> +		*f++ = '.';						\
> +	*f = 0;								\
>  	if ((family) == AF_INET) {					\
> -		net_info_ratelimited("%s for (%pI4, %d)->(%pI4, %d) %s" fmt "\n", \
> +		net_info_ratelimited("%s for %pI4.%d->%pI4.%d [%s] " fmt "\n", \
>  				msg, &ip_hdr(skb)->saddr, ntohs(th->source), \
>  				&ip_hdr(skb)->daddr, ntohs(th->dest),	\
>  				hdr_flags, ##__VA_ARGS__);		\
>  	} else {							\
> -		net_info_ratelimited("%s for [%pI6c]:%u->[%pI6c]:%u %s" fmt "\n", \
> +		net_info_ratelimited("%s for [%pI6c].%d->[%pI6c].%d [%s]" fmt "\n", \
>  				msg, &ipv6_hdr(skb)->saddr, ntohs(th->source), \
>  				&ipv6_hdr(skb)->daddr, ntohs(th->dest),	\
>  				hdr_flags, ##__VA_ARGS__);		\

-- 
Dmitry