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] [day] [month] [year] [list]
Message-ID: <CAMzD94Q+O3TTgwDUDJhHtg-reEGHZjaoPtGM-4V8k8bwR89NKA@mail.gmail.com>
Date: Tue, 26 Nov 2024 05:25:28 -0500
From: Brian Vazquez <brianvv@...gle.com>
To: Eric Dumazet <edumazet@...gle.com>
Cc: "David S . Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, 
	Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, eric.dumazet@...il.com, 
	syzbot+8b0959fc16551d55896b@...kaller.appspotmail.com, 
	Kuniyuki Iwashima <kuniyu@...zon.com>
Subject: Re: [PATCH net] tcp: populate XPS related fields of timewait sockets

Thanks for the patch!

On Mon, Nov 25, 2024 at 4:30 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> syzbot reported that netdev_core_pick_tx() was reading an unitialized
> field [1].
>
> This is indeed hapening for timewait sockets after recent commits.
>
> We can copy the original established socket sk_tx_queue_mapping
> and sk_rx_queue_mapping fields, instead of adding more checks
> in fast paths.
>
> As a bonus, packets will use the same transmit queue than
> prior ones, this potentially can avoid reordering.
>
> [1]
> BUG: KMSAN: uninit-value in netdev_pick_tx+0x5c7/0x1550
>  netdev_pick_tx+0x5c7/0x1550
>   netdev_core_pick_tx+0x1d2/0x4a0 net/core/dev.c:4312
>   __dev_queue_xmit+0x128a/0x57d0 net/core/dev.c:4394
>   dev_queue_xmit include/linux/netdevice.h:3168 [inline]
>   neigh_hh_output include/net/neighbour.h:523 [inline]
>   neigh_output include/net/neighbour.h:537 [inline]
>   ip_finish_output2+0x187c/0x1b70 net/ipv4/ip_output.c:236
>  __ip_finish_output+0x287/0x810
>   ip_finish_output+0x4b/0x600 net/ipv4/ip_output.c:324
>   NF_HOOK_COND include/linux/netfilter.h:303 [inline]
>   ip_output+0x15f/0x3f0 net/ipv4/ip_output.c:434
>   dst_output include/net/dst.h:450 [inline]
>   ip_local_out net/ipv4/ip_output.c:130 [inline]
>   ip_send_skb net/ipv4/ip_output.c:1505 [inline]
>   ip_push_pending_frames+0x444/0x570 net/ipv4/ip_output.c:1525
>   ip_send_unicast_reply+0x18c1/0x1b30 net/ipv4/ip_output.c:1672
>   tcp_v4_send_reset+0x238d/0x2a40 net/ipv4/tcp_ipv4.c:910
>   tcp_v4_rcv+0x48f8/0x5750 net/ipv4/tcp_ipv4.c:2431
>   ip_protocol_deliver_rcu+0x2a3/0x13d0 net/ipv4/ip_input.c:205
>   ip_local_deliver_finish+0x336/0x500 net/ipv4/ip_input.c:233
>   NF_HOOK include/linux/netfilter.h:314 [inline]
>   ip_local_deliver+0x21f/0x490 net/ipv4/ip_input.c:254
>   dst_input include/net/dst.h:460 [inline]
>   ip_sublist_rcv_finish net/ipv4/ip_input.c:578 [inline]
>   ip_list_rcv_finish net/ipv4/ip_input.c:628 [inline]
>   ip_sublist_rcv+0x15f3/0x17f0 net/ipv4/ip_input.c:636
>   ip_list_rcv+0x9ef/0xa40 net/ipv4/ip_input.c:670
>   __netif_receive_skb_list_ptype net/core/dev.c:5715 [inline]
>   __netif_receive_skb_list_core+0x15c5/0x1670 net/core/dev.c:5762
>   __netif_receive_skb_list net/core/dev.c:5814 [inline]
>   netif_receive_skb_list_internal+0x1085/0x1700 net/core/dev.c:5905
>   gro_normal_list include/net/gro.h:515 [inline]
>   napi_complete_done+0x3d4/0x810 net/core/dev.c:6256
>   virtqueue_napi_complete drivers/net/virtio_net.c:758 [inline]
>   virtnet_poll+0x5d80/0x6bf0 drivers/net/virtio_net.c:3013
>   __napi_poll+0xe7/0x980 net/core/dev.c:6877
>   napi_poll net/core/dev.c:6946 [inline]
>   net_rx_action+0xa5a/0x19b0 net/core/dev.c:7068
>   handle_softirqs+0x1a0/0x7c0 kernel/softirq.c:554
>   __do_softirq kernel/softirq.c:588 [inline]
>   invoke_softirq kernel/softirq.c:428 [inline]
>   __irq_exit_rcu+0x68/0x180 kernel/softirq.c:655
>   irq_exit_rcu+0x12/0x20 kernel/softirq.c:671
>   common_interrupt+0x97/0xb0 arch/x86/kernel/irq.c:278
>   asm_common_interrupt+0x2b/0x40 arch/x86/include/asm/idtentry.h:693
>   __preempt_count_sub arch/x86/include/asm/preempt.h:84 [inline]
>   kmsan_virt_addr_valid arch/x86/include/asm/kmsan.h:95 [inline]
>   virt_to_page_or_null+0xfb/0x150 mm/kmsan/shadow.c:75
>   kmsan_get_metadata+0x13e/0x1c0 mm/kmsan/shadow.c:141
>   kmsan_get_shadow_origin_ptr+0x4d/0xb0 mm/kmsan/shadow.c:102
>   get_shadow_origin_ptr mm/kmsan/instrumentation.c:38 [inline]
>   __msan_metadata_ptr_for_store_4+0x27/0x40 mm/kmsan/instrumentation.c:93
>   rcu_preempt_read_enter kernel/rcu/tree_plugin.h:390 [inline]
>   __rcu_read_lock+0x46/0x70 kernel/rcu/tree_plugin.h:413
>   rcu_read_lock include/linux/rcupdate.h:847 [inline]
>   batadv_nc_purge_orig_hash net/batman-adv/network-coding.c:408 [inline]
>   batadv_nc_worker+0x114/0x19e0 net/batman-adv/network-coding.c:719
>   process_one_work kernel/workqueue.c:3229 [inline]
>   process_scheduled_works+0xae0/0x1c40 kernel/workqueue.c:3310
>   worker_thread+0xea7/0x14f0 kernel/workqueue.c:3391
>   kthread+0x3e2/0x540 kernel/kthread.c:389
>   ret_from_fork+0x6d/0x90 arch/x86/kernel/process.c:147
>   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244
>
> Uninit was created at:
>   __alloc_pages_noprof+0x9a7/0xe00 mm/page_alloc.c:4774
>   alloc_pages_mpol_noprof+0x299/0x990 mm/mempolicy.c:2265
>   alloc_pages_noprof+0x1bf/0x1e0 mm/mempolicy.c:2344
>   alloc_slab_page mm/slub.c:2412 [inline]
>   allocate_slab+0x320/0x12e0 mm/slub.c:2578
>   new_slab mm/slub.c:2631 [inline]
>   ___slab_alloc+0x12ef/0x35e0 mm/slub.c:3818
>   __slab_alloc mm/slub.c:3908 [inline]
>   __slab_alloc_node mm/slub.c:3961 [inline]
>   slab_alloc_node mm/slub.c:4122 [inline]
>   kmem_cache_alloc_noprof+0x57a/0xb20 mm/slub.c:4141
>   inet_twsk_alloc+0x11f/0x9d0 net/ipv4/inet_timewait_sock.c:188
>   tcp_time_wait+0x83/0xf50 net/ipv4/tcp_minisocks.c:309
>  tcp_rcv_state_process+0x145a/0x49d0
>   tcp_v4_do_rcv+0xbf9/0x11a0 net/ipv4/tcp_ipv4.c:1939
>   tcp_v4_rcv+0x51df/0x5750 net/ipv4/tcp_ipv4.c:2351
>   ip_protocol_deliver_rcu+0x2a3/0x13d0 net/ipv4/ip_input.c:205
>   ip_local_deliver_finish+0x336/0x500 net/ipv4/ip_input.c:233
>   NF_HOOK include/linux/netfilter.h:314 [inline]
>   ip_local_deliver+0x21f/0x490 net/ipv4/ip_input.c:254
>   dst_input include/net/dst.h:460 [inline]
>   ip_sublist_rcv_finish net/ipv4/ip_input.c:578 [inline]
>   ip_list_rcv_finish net/ipv4/ip_input.c:628 [inline]
>   ip_sublist_rcv+0x15f3/0x17f0 net/ipv4/ip_input.c:636
>   ip_list_rcv+0x9ef/0xa40 net/ipv4/ip_input.c:670
>   __netif_receive_skb_list_ptype net/core/dev.c:5715 [inline]
>   __netif_receive_skb_list_core+0x15c5/0x1670 net/core/dev.c:5762
>   __netif_receive_skb_list net/core/dev.c:5814 [inline]
>   netif_receive_skb_list_internal+0x1085/0x1700 net/core/dev.c:5905
>   gro_normal_list include/net/gro.h:515 [inline]
>   napi_complete_done+0x3d4/0x810 net/core/dev.c:6256
>   virtqueue_napi_complete drivers/net/virtio_net.c:758 [inline]
>   virtnet_poll+0x5d80/0x6bf0 drivers/net/virtio_net.c:3013
>   __napi_poll+0xe7/0x980 net/core/dev.c:6877
>   napi_poll net/core/dev.c:6946 [inline]
>   net_rx_action+0xa5a/0x19b0 net/core/dev.c:7068
>   handle_softirqs+0x1a0/0x7c0 kernel/softirq.c:554
>   __do_softirq kernel/softirq.c:588 [inline]
>   invoke_softirq kernel/softirq.c:428 [inline]
>   __irq_exit_rcu+0x68/0x180 kernel/softirq.c:655
>   irq_exit_rcu+0x12/0x20 kernel/softirq.c:671
>   common_interrupt+0x97/0xb0 arch/x86/kernel/irq.c:278
>   asm_common_interrupt+0x2b/0x40 arch/x86/include/asm/idtentry.h:693
>
> CPU: 0 UID: 0 PID: 3962 Comm: kworker/u8:18 Not tainted 6.12.0-syzkaller-09073-g9f16d5e6f220 #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
> Workqueue: bat_events batadv_nc_worker
>
> Fixes: 79636038d37e ("ipv4: tcp: give socket pointer to control skbs")
> Fixes: 507a96737d99 ("ipv6: tcp: give socket pointer to control skbs")
> Reported-by: syzbot+8b0959fc16551d55896b@...kaller.appspotmail.com
> Link: https://lore.kernel.org/netdev/674442bd.050a0220.1cc393.0072.GAE@google.com/T/#u
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>

Reviewed-by: Brian Vazquez <brianvv@...gle.com>

> ---
> Cc: Kuniyuki Iwashima <kuniyu@...zon.com>
> Cc: Brian Vazquez <brianvv@...gle.com>
> ---
>  include/net/inet_timewait_sock.h | 2 ++
>  net/ipv4/tcp_minisocks.c         | 4 ++++
>  2 files changed, 6 insertions(+)
>
> diff --git a/include/net/inet_timewait_sock.h b/include/net/inet_timewait_sock.h
> index beb533a0e88098a95a1365b51bdc2d9e9dfd1d07..62c0a7e65d6bdf4c71a8ea90586b985f9fd30229 100644
> --- a/include/net/inet_timewait_sock.h
> +++ b/include/net/inet_timewait_sock.h
> @@ -45,6 +45,8 @@ struct inet_timewait_sock {
>  #define tw_node                        __tw_common.skc_nulls_node
>  #define tw_bind_node           __tw_common.skc_bind_node
>  #define tw_refcnt              __tw_common.skc_refcnt
> +#define tw_tx_queue_mapping    __tw_common.skc_tx_queue_mapping
> +#define tw_rx_queue_mapping    __tw_common.skc_rx_queue_mapping
>  #define tw_hash                        __tw_common.skc_hash
>  #define tw_prot                        __tw_common.skc_prot
>  #define tw_net                 __tw_common.skc_net
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index bb1fe1ba867ac3ed8610ceb9fef7e74cd465b3ea..7121d8573928cbf6840b3361b62f4812d365a30b 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -326,6 +326,10 @@ void tcp_time_wait(struct sock *sk, int state, int timeo)
>                 tcptw->tw_last_oow_ack_time = 0;
>                 tcptw->tw_tx_delay      = tp->tcp_tx_delay;
>                 tw->tw_txhash           = sk->sk_txhash;
> +               tw->tw_tx_queue_mapping = sk->sk_tx_queue_mapping;
> +#ifdef CONFIG_SOCK_RX_QUEUE_MAPPING
> +               tw->tw_rx_queue_mapping = sk->sk_rx_queue_mapping;
> +#endif
>  #if IS_ENABLED(CONFIG_IPV6)
>                 if (tw->tw_family == PF_INET6) {
>                         struct ipv6_pinfo *np = inet6_sk(sk);
> --
> 2.47.0.371.ga323438b13-goog
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ