| 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
| ||
|
Date: Wed, 1 Nov 2023 00:02:47 +0000
From: Dmitry Safonov <dima@...sta.com>
To: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
<davem@...emloft.net>
Cc: netdev@...r.kernel.org, eric.dumazet@...il.com,
syzbot <syzkaller@...glegroups.com>, Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>,
Francesco Ruggeri <fruggeri05@...il.com>,
Salam Noureddine <noureddine@...sta.com>
Subject: Re: [PATCH net] net/tcp: fix possible out-of-bounds reads in
tcp_hash_fail()
Hi Eric,
On 10/30/23 17:47, Dmitry Safonov wrote:
> On 10/30/23 08:45, Eric Dumazet wrote:
>> syzbot managed to trigger a fault by sending TCP packets
>> with all flags being set.
>>
>> 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>
>
> Thanks for fixing this,
> Reviewed-by: Dmitry Safonov <dima@...sta.com>
>
>> ---
>> include/net/tcp_ao.h | 5 ++---
>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/net/tcp_ao.h b/include/net/tcp_ao.h
>> index a375a171ef3cb37ab1d8246c72c6a3e83f5c9184..5daf96a3dbee14bd3786e19ea4972e351058e6e7 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[5]; \
>> char *f = hdr_flags; \
>> \
>> if (th->fin) \
>> @@ -135,8 +135,7 @@ do { \
>> *f++ = 'R'; \
>> if (th->ack) \
>> *f++ = 'A'; \
>> - if (f != hdr_flags) \
>> - *f = ' '; \
> Ah, that was clearly typo: I meant "*f = 0;"
Actually, after testing, I can see that the space was intended as well,
otherwise it "sticks" to L3index:
[ 130.965652] TCP: AO key not found for (10.0.1.1, 56920)->(10.0.254.1,
7010) Skeyid: 100 L3index: 0
[ 131.975116] TCP: AO hash is required, but not found for (10.0.1.1,
52686)->(10.0.254.1, 7011) SL3 index 0
[ 132.984024] TCP: AO hash mismatch for (10.0.1.1, 51382)->(10.0.254.1,
7012) SL3index: 0
[ 133.992221] TCP: Requested by the peer AO key id not found for
(10.0.1.1, 36548)->(10.0.254.1, 7013) SL3index: 0
If you don't mind, I'll send an updated version of your patch together
with some other small post-merge fixes this week.
>
>> + *f = 0; \
>> if ((family) == AF_INET) { \
>> net_info_ratelimited("%s for (%pI4, %d)->(%pI4, %d) %s" fmt "\n", \
>> msg, &ip_hdr(skb)->saddr, ntohs(th->source), \
>
Thank you,
Dmitry
Powered by blists - more mailing lists