From b1fa60ec252fba39130107074becd12d0b3f83ec Mon Sep 17 00:00:00 2001 From: Florian Fainelli Date: Thu, 16 May 2024 09:56:30 -0700 Subject: [PATCH 01/25] net: Always descend into dsa/ folder with CONFIG_NET_DSA enabled Stephen reported that he was unable to get the dsa_loop driver to get probed, and the reason ended up being because he had CONFIG_FIXED_PHY=y in his kernel configuration. As Masahiro explained it: "obj-m += dsa/" means everything under dsa/ must be modular. If there is a built-in object under dsa/ with CONFIG_NET_DSA=m, you cannot do "obj-$(CONFIG_NET_DSA) += dsa/". You need to change it back to "obj-y += dsa/". This was the case here whereby CONFIG_NET_DSA=m, and so the obj-$(CONFIG_FIXED_PHY) += dsa_loop_bdinfo.o rule is not executed and the DSA loop mdio_board info structure is not registered with the kernel, and eventually the device is simply not found. To preserve the intention of the original commit of limiting the amount of folder descending, conditionally descend into drivers/net/dsa when CONFIG_NET_DSA is enabled. Fixes: 227d72063fcc ("dsa: simplify Kconfig symbols and dependencies") Reported-by: Stephen Langstaff Signed-off-by: Florian Fainelli Reviewed-by: Vladimir Oltean Reviewed-by: Alexander Lobakin Signed-off-by: David S. Miller --- drivers/net/Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 9c053673d6b2..13743d0e83b5 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -49,7 +49,9 @@ obj-$(CONFIG_MHI_NET) += mhi_net.o obj-$(CONFIG_ARCNET) += arcnet/ obj-$(CONFIG_CAIF) += caif/ obj-$(CONFIG_CAN) += can/ -obj-$(CONFIG_NET_DSA) += dsa/ +ifdef CONFIG_NET_DSA +obj-y += dsa/ +endif obj-$(CONFIG_ETHERNET) += ethernet/ obj-$(CONFIG_FDDI) += fddi/ obj-$(CONFIG_HIPPI) += hippi/ From 5447f9708d9e4c17a647b16a9cb29e9e02820bd9 Mon Sep 17 00:00:00 2001 From: Andrea Mayer Date: Fri, 17 May 2024 18:45:41 +0200 Subject: [PATCH 02/25] ipv6: sr: fix missing sk_buff release in seg6_input_core The seg6_input() function is responsible for adding the SRH into a packet, delegating the operation to the seg6_input_core(). This function uses the skb_cow_head() to ensure that there is sufficient headroom in the sk_buff for accommodating the link-layer header. In the event that the skb_cow_header() function fails, the seg6_input_core() catches the error but it does not release the sk_buff, which will result in a memory leak. This issue was introduced in commit af3b5158b89d ("ipv6: sr: fix BUG due to headroom too small after SRH push") and persists even after commit 7a3f5b0de364 ("netfilter: add netfilter hooks to SRv6 data plane"), where the entire seg6_input() code was refactored to deal with netfilter hooks. The proposed patch addresses the identified memory leak by requiring the seg6_input_core() function to release the sk_buff in the event that skb_cow_head() fails. Fixes: af3b5158b89d ("ipv6: sr: fix BUG due to headroom too small after SRH push") Signed-off-by: Andrea Mayer Reviewed-by: Simon Horman Reviewed-by: David Ahern Signed-off-by: David S. Miller --- net/ipv6/seg6_iptunnel.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/net/ipv6/seg6_iptunnel.c b/net/ipv6/seg6_iptunnel.c index 03b877ff4558..a75df2ec8db0 100644 --- a/net/ipv6/seg6_iptunnel.c +++ b/net/ipv6/seg6_iptunnel.c @@ -459,10 +459,8 @@ static int seg6_input_core(struct net *net, struct sock *sk, int err; err = seg6_do_srh(skb); - if (unlikely(err)) { - kfree_skb(skb); - return err; - } + if (unlikely(err)) + goto drop; slwt = seg6_lwt_lwtunnel(orig_dst->lwtstate); @@ -486,7 +484,7 @@ static int seg6_input_core(struct net *net, struct sock *sk, err = skb_cow_head(skb, LL_RESERVED_SPACE(dst->dev)); if (unlikely(err)) - return err; + goto drop; if (static_branch_unlikely(&nf_hooks_lwtunnel_enabled)) return NF_HOOK(NFPROTO_IPV6, NF_INET_LOCAL_OUT, @@ -494,6 +492,9 @@ static int seg6_input_core(struct net *net, struct sock *sk, skb_dst(skb)->dev, seg6_input_finish); return seg6_input_finish(dev_net(skb->dev), NULL, skb); +drop: + kfree_skb(skb); + return err; } static int seg6_input_nf(struct sk_buff *skb) From cc563e749810f5636451d4b833fbd689899ecdb9 Mon Sep 17 00:00:00 2001 From: Taehee Yoo Date: Sat, 18 May 2024 13:20:52 +0000 Subject: [PATCH 03/25] selftests: net: kill smcrouted in the cleanup logic in amt.sh The amt.sh requires smcrouted for multicasting routing. So, it starts smcrouted before forwarding tests. It must be stopped after all tests, but it isn't. To fix this issue, it kills smcrouted in the cleanup logic. Fixes: c08e8baea78e ("selftests: add amt interface selftest script") Signed-off-by: Taehee Yoo Reviewed-by: Simon Horman Signed-off-by: David S. Miller --- tools/testing/selftests/net/amt.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/net/amt.sh b/tools/testing/selftests/net/amt.sh index 5175a42cbe8a..7e7ed6c558da 100755 --- a/tools/testing/selftests/net/amt.sh +++ b/tools/testing/selftests/net/amt.sh @@ -77,6 +77,7 @@ readonly LISTENER=$(mktemp -u listener-XXXXXXXX) readonly GATEWAY=$(mktemp -u gateway-XXXXXXXX) readonly RELAY=$(mktemp -u relay-XXXXXXXX) readonly SOURCE=$(mktemp -u source-XXXXXXXX) +readonly SMCROUTEDIR="$(mktemp -d)" ERR=4 err=0 @@ -85,6 +86,11 @@ exit_cleanup() for ns in "$@"; do ip netns delete "${ns}" 2>/dev/null || true done + if [ -f "$SMCROUTEDIR/amt.pid" ]; then + smcpid=$(< $SMCROUTEDIR/amt.pid) + kill $smcpid + fi + rm -rf $SMCROUTEDIR exit $ERR } @@ -167,7 +173,7 @@ setup_iptables() setup_mcast_routing() { - ip netns exec "${RELAY}" smcrouted + ip netns exec "${RELAY}" smcrouted -P $SMCROUTEDIR/amt.pid ip netns exec "${RELAY}" smcroutectl a relay_src \ 172.17.0.2 239.0.0.1 amtr ip netns exec "${RELAY}" smcroutectl a relay_src \ From e4a87abf588536d1cdfb128595e6e680af5cf3ed Mon Sep 17 00:00:00 2001 From: Ryosuke Yasuoka Date: Sun, 19 May 2024 18:43:03 +0900 Subject: [PATCH 04/25] nfc: nci: Fix uninit-value in nci_rx_work syzbot reported the following uninit-value access issue [1] nci_rx_work() parses received packet from ndev->rx_q. It should be validated header size, payload size and total packet size before processing the packet. If an invalid packet is detected, it should be silently discarded. Fixes: d24b03535e5e ("nfc: nci: Fix uninit-value in nci_dev_up and nci_ntf_packet") Reported-and-tested-by: syzbot+d7b4dc6cd50410152534@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=d7b4dc6cd50410152534 [1] Signed-off-by: Ryosuke Yasuoka Reviewed-by: Krzysztof Kozlowski Signed-off-by: David S. Miller --- net/nfc/nci/core.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c index b133dc55304c..7a9897fbf4f4 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -1463,6 +1463,19 @@ int nci_core_ntf_packet(struct nci_dev *ndev, __u16 opcode, ndev->ops->n_core_ops); } +static bool nci_valid_size(struct sk_buff *skb) +{ + BUILD_BUG_ON(NCI_CTRL_HDR_SIZE != NCI_DATA_HDR_SIZE); + unsigned int hdr_size = NCI_CTRL_HDR_SIZE; + + if (skb->len < hdr_size || + !nci_plen(skb->data) || + skb->len < hdr_size + nci_plen(skb->data)) { + return false; + } + return true; +} + /* ---- NCI TX Data worker thread ---- */ static void nci_tx_work(struct work_struct *work) @@ -1516,7 +1529,7 @@ static void nci_rx_work(struct work_struct *work) nfc_send_to_raw_sock(ndev->nfc_dev, skb, RAW_PAYLOAD_NCI, NFC_DIRECTION_RX); - if (!nci_plen(skb->data)) { + if (!nci_valid_size(skb)) { kfree_skb(skb); kcov_remote_stop(); break; From eabb8a9be1e4a12f3bf37ceb7411083e3775672d Mon Sep 17 00:00:00 2001 From: Heiner Kallweit Date: Wed, 15 May 2024 08:18:01 +0200 Subject: [PATCH 05/25] Revert "r8169: don't try to disable interrupts if NAPI is, scheduled already" This reverts commit 7274c4147afbf46f45b8501edbdad6da8cd013b9. Ken reported that RTL8125b can lock up if gro_flush_timeout has the default value of 20000 and napi_defer_hard_irqs is set to 0. In this scenario device interrupts aren't disabled, what seems to trigger some silicon bug under heavy load. I was able to reproduce this behavior on RTL8168h. Fix this by reverting 7274c4147afb. Fixes: 7274c4147afb ("r8169: don't try to disable interrupts if NAPI is scheduled already") Cc: stable@vger.kernel.org Reported-by: Ken Milmore Signed-off-by: Heiner Kallweit Reviewed-by: Eric Dumazet Link: https://lore.kernel.org/r/9b5b6f4c-4f54-4b90-b0b3-8d8023c2e780@gmail.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/realtek/r8169_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 5abbea91bc07..86a6d4225bc2 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -4657,10 +4657,8 @@ static irqreturn_t rtl8169_interrupt(int irq, void *dev_instance) rtl_schedule_task(tp, RTL_FLAG_TASK_RESET_PENDING); } - if (napi_schedule_prep(&tp->napi)) { - rtl_irq_disable(tp); - __napi_schedule(&tp->napi); - } + rtl_irq_disable(tp); + napi_schedule(&tp->napi); out: rtl_ack_events(tp, status); From 9841991a446c87f90f66f4b9fee6fe934c1336a2 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Thu, 16 May 2024 22:48:35 +0900 Subject: [PATCH 06/25] af_unix: Update unix_sk(sk)->oob_skb under sk_receive_queue lock. Billy Jheng Bing-Jhong reported a race between __unix_gc() and queue_oob(). __unix_gc() tries to garbage-collect close()d inflight sockets, and then if the socket has MSG_OOB in unix_sk(sk)->oob_skb, GC will drop the reference and set NULL to it locklessly. However, the peer socket still can send MSG_OOB message and queue_oob() can update unix_sk(sk)->oob_skb concurrently, leading NULL pointer dereference. [0] To fix the issue, let's update unix_sk(sk)->oob_skb under the sk_receive_queue's lock and take it everywhere we touch oob_skb. Note that we defer kfree_skb() in manage_oob() to silence lockdep false-positive (See [1]). [0]: BUG: kernel NULL pointer dereference, address: 0000000000000008 PF: supervisor write access in kernel mode PF: error_code(0x0002) - not-present page PGD 8000000009f5e067 P4D 8000000009f5e067 PUD 9f5d067 PMD 0 Oops: 0002 [#1] PREEMPT SMP PTI CPU: 3 PID: 50 Comm: kworker/3:1 Not tainted 6.9.0-rc5-00191-gd091e579b864 #110 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014 Workqueue: events delayed_fput RIP: 0010:skb_dequeue (./include/linux/skbuff.h:2386 ./include/linux/skbuff.h:2402 net/core/skbuff.c:3847) Code: 39 e3 74 3e 8b 43 10 48 89 ef 83 e8 01 89 43 10 49 8b 44 24 08 49 c7 44 24 08 00 00 00 00 49 8b 14 24 49 c7 04 24 00 00 00 00 <48> 89 42 08 48 89 10 e8 e7 c5 42 00 4c 89 e0 5b 5d 41 5c c3 cc cc RSP: 0018:ffffc900001bfd48 EFLAGS: 00000002 RAX: 0000000000000000 RBX: ffff8880088f5ae8 RCX: 00000000361289f9 RDX: 0000000000000000 RSI: 0000000000000206 RDI: ffff8880088f5b00 RBP: ffff8880088f5b00 R08: 0000000000080000 R09: 0000000000000001 R10: 0000000000000003 R11: 0000000000000001 R12: ffff8880056b6a00 R13: ffff8880088f5280 R14: 0000000000000001 R15: ffff8880088f5a80 FS: 0000000000000000(0000) GS:ffff88807dd80000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000008 CR3: 0000000006314000 CR4: 00000000007506f0 PKRU: 55555554 Call Trace: unix_release_sock (net/unix/af_unix.c:654) unix_release (net/unix/af_unix.c:1050) __sock_release (net/socket.c:660) sock_close (net/socket.c:1423) __fput (fs/file_table.c:423) delayed_fput (fs/file_table.c:444 (discriminator 3)) process_one_work (kernel/workqueue.c:3259) worker_thread (kernel/workqueue.c:3329 kernel/workqueue.c:3416) kthread (kernel/kthread.c:388) ret_from_fork (arch/x86/kernel/process.c:153) ret_from_fork_asm (arch/x86/entry/entry_64.S:257) Modules linked in: CR2: 0000000000000008 Link: https://lore.kernel.org/netdev/a00d3993-c461-43f2-be6d-07259c98509a@rbox.co/ [1] Fixes: 1279f9d9dec2 ("af_unix: Call kfree_skb() for dead unix_(sk)->oob_skb in GC.") Reported-by: Billy Jheng Bing-Jhong Signed-off-by: Kuniyuki Iwashima Link: https://lore.kernel.org/r/20240516134835.8332-1-kuniyu@amazon.com Signed-off-by: Paolo Abeni --- net/unix/af_unix.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c index fa906ec5e657..ca101690e740 100644 --- a/net/unix/af_unix.c +++ b/net/unix/af_unix.c @@ -2171,13 +2171,15 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other maybe_add_creds(skb, sock, other); skb_get(skb); + scm_stat_add(other, skb); + + spin_lock(&other->sk_receive_queue.lock); if (ousk->oob_skb) consume_skb(ousk->oob_skb); - WRITE_ONCE(ousk->oob_skb, skb); + __skb_queue_tail(&other->sk_receive_queue, skb); + spin_unlock(&other->sk_receive_queue.lock); - scm_stat_add(other, skb); - skb_queue_tail(&other->sk_receive_queue, skb); sk_send_sigurg(other); unix_state_unlock(other); other->sk_data_ready(other); @@ -2568,8 +2570,10 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state) mutex_lock(&u->iolock); unix_state_lock(sk); + spin_lock(&sk->sk_receive_queue.lock); if (sock_flag(sk, SOCK_URGINLINE) || !u->oob_skb) { + spin_unlock(&sk->sk_receive_queue.lock); unix_state_unlock(sk); mutex_unlock(&u->iolock); return -EINVAL; @@ -2581,6 +2585,8 @@ static int unix_stream_recv_urg(struct unix_stream_read_state *state) WRITE_ONCE(u->oob_skb, NULL); else skb_get(oob_skb); + + spin_unlock(&sk->sk_receive_queue.lock); unix_state_unlock(sk); chunk = state->recv_actor(oob_skb, 0, chunk, state); @@ -2609,6 +2615,10 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk, consume_skb(skb); skb = NULL; } else { + struct sk_buff *unlinked_skb = NULL; + + spin_lock(&sk->sk_receive_queue.lock); + if (skb == u->oob_skb) { if (copied) { skb = NULL; @@ -2620,13 +2630,19 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk, } else if (flags & MSG_PEEK) { skb = NULL; } else { - skb_unlink(skb, &sk->sk_receive_queue); + __skb_unlink(skb, &sk->sk_receive_queue); WRITE_ONCE(u->oob_skb, NULL); - if (!WARN_ON_ONCE(skb_unref(skb))) - kfree_skb(skb); + unlinked_skb = skb; skb = skb_peek(&sk->sk_receive_queue); } } + + spin_unlock(&sk->sk_receive_queue.lock); + + if (unlinked_skb) { + WARN_ON_ONCE(skb_unref(unlinked_skb)); + kfree_skb(unlinked_skb); + } } return skb; } From efb9f4f19f8e37fde43dfecebc80292d179f56c6 Mon Sep 17 00:00:00 2001 From: Hangbin Liu Date: Fri, 17 May 2024 08:54:35 +0800 Subject: [PATCH 07/25] ipv6: sr: fix memleak in seg6_hmac_init_algo seg6_hmac_init_algo returns without cleaning up the previous allocations if one fails, so it's going to leak all that memory and the crypto tfms. Update seg6_hmac_exit to only free the memory when allocated, so we can reuse the code directly. Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support") Reported-by: Sabrina Dubroca Closes: https://lore.kernel.org/netdev/Zj3bh-gE7eT6V6aH@hog/ Signed-off-by: Hangbin Liu Reviewed-by: Simon Horman Reviewed-by: Sabrina Dubroca Link: https://lore.kernel.org/r/20240517005435.2600277-1-liuhangbin@gmail.com Signed-off-by: Paolo Abeni --- net/ipv6/seg6_hmac.c | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c index 861e0366f549..bbf5b84a70fc 100644 --- a/net/ipv6/seg6_hmac.c +++ b/net/ipv6/seg6_hmac.c @@ -356,6 +356,7 @@ static int seg6_hmac_init_algo(void) struct crypto_shash *tfm; struct shash_desc *shash; int i, alg_count, cpu; + int ret = -ENOMEM; alg_count = ARRAY_SIZE(hmac_algos); @@ -366,12 +367,14 @@ static int seg6_hmac_init_algo(void) algo = &hmac_algos[i]; algo->tfms = alloc_percpu(struct crypto_shash *); if (!algo->tfms) - return -ENOMEM; + goto error_out; for_each_possible_cpu(cpu) { tfm = crypto_alloc_shash(algo->name, 0, 0); - if (IS_ERR(tfm)) - return PTR_ERR(tfm); + if (IS_ERR(tfm)) { + ret = PTR_ERR(tfm); + goto error_out; + } p_tfm = per_cpu_ptr(algo->tfms, cpu); *p_tfm = tfm; } @@ -383,18 +386,22 @@ static int seg6_hmac_init_algo(void) algo->shashs = alloc_percpu(struct shash_desc *); if (!algo->shashs) - return -ENOMEM; + goto error_out; for_each_possible_cpu(cpu) { shash = kzalloc_node(shsize, GFP_KERNEL, cpu_to_node(cpu)); if (!shash) - return -ENOMEM; + goto error_out; *per_cpu_ptr(algo->shashs, cpu) = shash; } } return 0; + +error_out: + seg6_hmac_exit(); + return ret; } int __init seg6_hmac_init(void) @@ -412,22 +419,29 @@ int __net_init seg6_hmac_net_init(struct net *net) void seg6_hmac_exit(void) { struct seg6_hmac_algo *algo = NULL; + struct crypto_shash *tfm; + struct shash_desc *shash; int i, alg_count, cpu; alg_count = ARRAY_SIZE(hmac_algos); for (i = 0; i < alg_count; i++) { algo = &hmac_algos[i]; - for_each_possible_cpu(cpu) { - struct crypto_shash *tfm; - struct shash_desc *shash; - shash = *per_cpu_ptr(algo->shashs, cpu); - kfree(shash); - tfm = *per_cpu_ptr(algo->tfms, cpu); - crypto_free_shash(tfm); + if (algo->shashs) { + for_each_possible_cpu(cpu) { + shash = *per_cpu_ptr(algo->shashs, cpu); + kfree(shash); + } + free_percpu(algo->shashs); + } + + if (algo->tfms) { + for_each_possible_cpu(cpu) { + tfm = *per_cpu_ptr(algo->tfms, cpu); + crypto_free_shash(tfm); + } + free_percpu(algo->tfms); } - free_percpu(algo->tfms); - free_percpu(algo->shashs); } } EXPORT_SYMBOL(seg6_hmac_exit); From ea63ac14292564eefc7dffe868ed354ff9ed6f4b Mon Sep 17 00:00:00 2001 From: Hangbin Liu Date: Fri, 17 May 2024 09:03:27 +0800 Subject: [PATCH 08/25] selftests/net: use tc rule to filter the na packet Test arp_ndisc_untracked_subnets use tcpdump to filter the unsolicited and untracked na messages. It set -e before calling tcpdump. But if tcpdump filters 0 packet, it will return none zero, and cause the script to exit. Instead of using slow tcpdump to capture packets, let's using tc rule to filter out the na message. At the same time, fix function setup_v6 which only needs one parameter. Move all the related helpers from forwarding lib.sh to net lib.sh. Fixes: 0ea7b0a454ca ("selftests: net: arp_ndisc_untracked_subnets: test for arp_accept and accept_untracked_na") Signed-off-by: Hangbin Liu Reviewed-by: Simon Horman Link: https://lore.kernel.org/r/20240517010327.2631319-1-liuhangbin@gmail.com Signed-off-by: Paolo Abeni --- .../net/arp_ndisc_untracked_subnets.sh | 53 ++++++----------- tools/testing/selftests/net/forwarding/lib.sh | 58 ------------------- tools/testing/selftests/net/lib.sh | 58 +++++++++++++++++++ 3 files changed, 75 insertions(+), 94 deletions(-) diff --git a/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh b/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh index a40c0e9bd023..eef5cbf6eecc 100755 --- a/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh +++ b/tools/testing/selftests/net/arp_ndisc_untracked_subnets.sh @@ -73,25 +73,19 @@ setup_v6() { # namespaces. veth0 is veth-router, veth1 is veth-host. # first, set up the inteface's link to the namespace # then, set the interface "up" - ip -6 -netns ${ROUTER_NS_V6} link add name ${ROUTER_INTF} \ - type veth peer name ${HOST_INTF} + ip -n ${ROUTER_NS_V6} link add name ${ROUTER_INTF} \ + type veth peer name ${HOST_INTF} netns ${HOST_NS_V6} - ip -6 -netns ${ROUTER_NS_V6} link set dev ${ROUTER_INTF} up - ip -6 -netns ${ROUTER_NS_V6} link set dev ${HOST_INTF} netns \ - ${HOST_NS_V6} - - ip -6 -netns ${HOST_NS_V6} link set dev ${HOST_INTF} up - ip -6 -netns ${ROUTER_NS_V6} addr add \ - ${ROUTER_ADDR_V6}/${PREFIX_WIDTH_V6} dev ${ROUTER_INTF} nodad + # Add tc rule to filter out host na message + tc -n ${ROUTER_NS_V6} qdisc add dev ${ROUTER_INTF} clsact + tc -n ${ROUTER_NS_V6} filter add dev ${ROUTER_INTF} \ + ingress protocol ipv6 pref 1 handle 101 \ + flower src_ip ${HOST_ADDR_V6} ip_proto icmpv6 type 136 skip_hw action pass HOST_CONF=net.ipv6.conf.${HOST_INTF} ip netns exec ${HOST_NS_V6} sysctl -qw ${HOST_CONF}.ndisc_notify=1 ip netns exec ${HOST_NS_V6} sysctl -qw ${HOST_CONF}.disable_ipv6=0 - ip -6 -netns ${HOST_NS_V6} addr add ${HOST_ADDR_V6}/${PREFIX_WIDTH_V6} \ - dev ${HOST_INTF} - ROUTER_CONF=net.ipv6.conf.${ROUTER_INTF} - ip netns exec ${ROUTER_NS_V6} sysctl -w \ ${ROUTER_CONF}.forwarding=1 >/dev/null 2>&1 ip netns exec ${ROUTER_NS_V6} sysctl -w \ @@ -99,6 +93,13 @@ setup_v6() { ip netns exec ${ROUTER_NS_V6} sysctl -w \ ${ROUTER_CONF}.accept_untracked_na=${accept_untracked_na} \ >/dev/null 2>&1 + + ip -n ${ROUTER_NS_V6} link set dev ${ROUTER_INTF} up + ip -n ${HOST_NS_V6} link set dev ${HOST_INTF} up + ip -n ${ROUTER_NS_V6} addr add ${ROUTER_ADDR_V6}/${PREFIX_WIDTH_V6} \ + dev ${ROUTER_INTF} nodad + ip -n ${HOST_NS_V6} addr add ${HOST_ADDR_V6}/${PREFIX_WIDTH_V6} \ + dev ${HOST_INTF} set +e } @@ -162,26 +163,6 @@ arp_test_gratuitous_combinations() { arp_test_gratuitous 2 1 } -cleanup_tcpdump() { - set -e - [[ ! -z ${tcpdump_stdout} ]] && rm -f ${tcpdump_stdout} - [[ ! -z ${tcpdump_stderr} ]] && rm -f ${tcpdump_stderr} - tcpdump_stdout= - tcpdump_stderr= - set +e -} - -start_tcpdump() { - set -e - tcpdump_stdout=`mktemp` - tcpdump_stderr=`mktemp` - ip netns exec ${ROUTER_NS_V6} timeout 15s \ - tcpdump --immediate-mode -tpni ${ROUTER_INTF} -c 1 \ - "icmp6 && icmp6[0] == 136 && src ${HOST_ADDR_V6}" \ - > ${tcpdump_stdout} 2> /dev/null - set +e -} - verify_ndisc() { local accept_untracked_na=$1 local same_subnet=$2 @@ -222,8 +203,9 @@ ndisc_test_untracked_advertisements() { HOST_ADDR_V6=2001:db8:abcd:0012::3 fi fi - setup_v6 $1 $2 - start_tcpdump + setup_v6 $1 + slowwait_for_counter 15 1 \ + tc_rule_handle_stats_get "dev ${ROUTER_INTF} ingress" 101 ".packets" "-n ${ROUTER_NS_V6}" if verify_ndisc $1 $2; then printf " TEST: %-60s [ OK ]\n" "${test_msg[*]}" @@ -231,7 +213,6 @@ ndisc_test_untracked_advertisements() { printf " TEST: %-60s [FAIL]\n" "${test_msg[*]}" fi - cleanup_tcpdump cleanup_v6 set +e } diff --git a/tools/testing/selftests/net/forwarding/lib.sh b/tools/testing/selftests/net/forwarding/lib.sh index 112c85c35092..eabbdf00d8ca 100644 --- a/tools/testing/selftests/net/forwarding/lib.sh +++ b/tools/testing/selftests/net/forwarding/lib.sh @@ -129,14 +129,6 @@ fi source "$net_forwarding_dir/../lib.sh" -# timeout in seconds -slowwait() -{ - local timeout_sec=$1; shift - - loopy_wait "sleep 0.1" "$((timeout_sec * 1000))" "$@" -} - ############################################################################## # Sanity checks @@ -678,33 +670,6 @@ wait_for_trap() "$@" | grep -q trap } -until_counter_is() -{ - local expr=$1; shift - local current=$("$@") - - echo $((current)) - ((current $expr)) -} - -busywait_for_counter() -{ - local timeout=$1; shift - local delta=$1; shift - - local base=$("$@") - busywait "$timeout" until_counter_is ">= $((base + delta))" "$@" -} - -slowwait_for_counter() -{ - local timeout=$1; shift - local delta=$1; shift - - local base=$("$@") - slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@" -} - setup_wait_dev() { local dev=$1; shift @@ -1023,29 +988,6 @@ link_stats_rx_errors_get() link_stats_get $1 rx errors } -tc_rule_stats_get() -{ - local dev=$1; shift - local pref=$1; shift - local dir=$1; shift - local selector=${1:-.packets}; shift - - tc -j -s filter show dev $dev ${dir:-ingress} pref $pref \ - | jq ".[1].options.actions[].stats$selector" -} - -tc_rule_handle_stats_get() -{ - local id=$1; shift - local handle=$1; shift - local selector=${1:-.packets}; shift - local netns=${1:-""}; shift - - tc $netns -j -s filter show $id \ - | jq ".[] | select(.options.handle == $handle) | \ - .options.actions[0].stats$selector" -} - ethtool_stats_get() { local dev=$1; shift diff --git a/tools/testing/selftests/net/lib.sh b/tools/testing/selftests/net/lib.sh index 72b191e4e064..edc030e81a46 100644 --- a/tools/testing/selftests/net/lib.sh +++ b/tools/testing/selftests/net/lib.sh @@ -91,6 +91,41 @@ busywait() loopy_wait : "$timeout_ms" "$@" } +# timeout in seconds +slowwait() +{ + local timeout_sec=$1; shift + + loopy_wait "sleep 0.1" "$((timeout_sec * 1000))" "$@" +} + +until_counter_is() +{ + local expr=$1; shift + local current=$("$@") + + echo $((current)) + ((current $expr)) +} + +busywait_for_counter() +{ + local timeout=$1; shift + local delta=$1; shift + + local base=$("$@") + busywait "$timeout" until_counter_is ">= $((base + delta))" "$@" +} + +slowwait_for_counter() +{ + local timeout=$1; shift + local delta=$1; shift + + local base=$("$@") + slowwait "$timeout" until_counter_is ">= $((base + delta))" "$@" +} + cleanup_ns() { local ns="" @@ -150,3 +185,26 @@ setup_ns() done NS_LIST="$NS_LIST $ns_list" } + +tc_rule_stats_get() +{ + local dev=$1; shift + local pref=$1; shift + local dir=$1; shift + local selector=${1:-.packets}; shift + + tc -j -s filter show dev $dev ${dir:-ingress} pref $pref \ + | jq ".[1].options.actions[].stats$selector" +} + +tc_rule_handle_stats_get() +{ + local id=$1; shift + local handle=$1; shift + local selector=${1:-.packets}; shift + local netns=${1:-""}; shift + + tc $netns -j -s filter show $id \ + | jq ".[] | select(.options.handle == $handle) | \ + .options.actions[0].stats$selector" +} From 3ebc46ca8675de6378e3f8f40768e180bb8afa66 Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 17 May 2024 18:16:26 +0900 Subject: [PATCH 09/25] tcp: Fix shift-out-of-bounds in dctcp_update_alpha(). In dctcp_update_alpha(), we use a module parameter dctcp_shift_g as follows: alpha -= min_not_zero(alpha, alpha >> dctcp_shift_g); ... delivered_ce <<= (10 - dctcp_shift_g); It seems syzkaller started fuzzing module parameters and triggered shift-out-of-bounds [0] by setting 100 to dctcp_shift_g: memcpy((void*)0x20000080, "/sys/module/tcp_dctcp/parameters/dctcp_shift_g\000", 47); res = syscall(__NR_openat, /*fd=*/0xffffffffffffff9cul, /*file=*/0x20000080ul, /*flags=*/2ul, /*mode=*/0ul); memcpy((void*)0x20000000, "100\000", 4); syscall(__NR_write, /*fd=*/r[0], /*val=*/0x20000000ul, /*len=*/4ul); Let's limit the max value of dctcp_shift_g by param_set_uint_minmax(). With this patch: # echo 10 > /sys/module/tcp_dctcp/parameters/dctcp_shift_g # cat /sys/module/tcp_dctcp/parameters/dctcp_shift_g 10 # echo 11 > /sys/module/tcp_dctcp/parameters/dctcp_shift_g -bash: echo: write error: Invalid argument [0]: UBSAN: shift-out-of-bounds in net/ipv4/tcp_dctcp.c:143:12 shift exponent 100 is too large for 32-bit type 'u32' (aka 'unsigned int') CPU: 0 PID: 8083 Comm: syz-executor345 Not tainted 6.9.0-05151-g1b294a1f3561 #2 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:88 [inline] dump_stack_lvl+0x201/0x300 lib/dump_stack.c:114 ubsan_epilogue lib/ubsan.c:231 [inline] __ubsan_handle_shift_out_of_bounds+0x346/0x3a0 lib/ubsan.c:468 dctcp_update_alpha+0x540/0x570 net/ipv4/tcp_dctcp.c:143 tcp_in_ack_event net/ipv4/tcp_input.c:3802 [inline] tcp_ack+0x17b1/0x3bc0 net/ipv4/tcp_input.c:3948 tcp_rcv_state_process+0x57a/0x2290 net/ipv4/tcp_input.c:6711 tcp_v4_do_rcv+0x764/0xc40 net/ipv4/tcp_ipv4.c:1937 sk_backlog_rcv include/net/sock.h:1106 [inline] __release_sock+0x20f/0x350 net/core/sock.c:2983 release_sock+0x61/0x1f0 net/core/sock.c:3549 mptcp_subflow_shutdown+0x3d0/0x620 net/mptcp/protocol.c:2907 mptcp_check_send_data_fin+0x225/0x410 net/mptcp/protocol.c:2976 __mptcp_close+0x238/0xad0 net/mptcp/protocol.c:3072 mptcp_close+0x2a/0x1a0 net/mptcp/protocol.c:3127 inet_release+0x190/0x1f0 net/ipv4/af_inet.c:437 __sock_release net/socket.c:659 [inline] sock_close+0xc0/0x240 net/socket.c:1421 __fput+0x41b/0x890 fs/file_table.c:422 task_work_run+0x23b/0x300 kernel/task_work.c:180 exit_task_work include/linux/task_work.h:38 [inline] do_exit+0x9c8/0x2540 kernel/exit.c:878 do_group_exit+0x201/0x2b0 kernel/exit.c:1027 __do_sys_exit_group kernel/exit.c:1038 [inline] __se_sys_exit_group kernel/exit.c:1036 [inline] __x64_sys_exit_group+0x3f/0x40 kernel/exit.c:1036 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xe4/0x240 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x67/0x6f RIP: 0033:0x7f6c2b5005b6 Code: Unable to access opcode bytes at 0x7f6c2b50058c. RSP: 002b:00007ffe883eb948 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7 RAX: ffffffffffffffda RBX: 00007f6c2b5862f0 RCX: 00007f6c2b5005b6 RDX: 0000000000000001 RSI: 000000000000003c RDI: 0000000000000001 RBP: 0000000000000001 R08: 00000000000000e7 R09: ffffffffffffffc0 R10: 0000000000000006 R11: 0000000000000246 R12: 00007f6c2b5862f0 R13: 0000000000000001 R14: 0000000000000000 R15: 0000000000000001 Reported-by: syzkaller Reported-by: Yue Sun Reported-by: xingwei lee Closes: https://lore.kernel.org/netdev/CAEkJfYNJM=cw-8x7_Vmj1J6uYVCWMbbvD=EFmDPVBGpTsqOxEA@mail.gmail.com/ Fixes: e3118e8359bb ("net: tcp: add DCTCP congestion control algorithm") Signed-off-by: Kuniyuki Iwashima Reviewed-by: Simon Horman Link: https://lore.kernel.org/r/20240517091626.32772-1-kuniyu@amazon.com Signed-off-by: Paolo Abeni --- net/ipv4/tcp_dctcp.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/net/ipv4/tcp_dctcp.c b/net/ipv4/tcp_dctcp.c index 6b712a33d49f..8a45a4aea933 100644 --- a/net/ipv4/tcp_dctcp.c +++ b/net/ipv4/tcp_dctcp.c @@ -58,7 +58,18 @@ struct dctcp { }; static unsigned int dctcp_shift_g __read_mostly = 4; /* g = 1/2^4 */ -module_param(dctcp_shift_g, uint, 0644); + +static int dctcp_shift_g_set(const char *val, const struct kernel_param *kp) +{ + return param_set_uint_minmax(val, kp, 0, 10); +} + +static const struct kernel_param_ops dctcp_shift_g_ops = { + .set = dctcp_shift_g_set, + .get = param_get_uint, +}; + +module_param_cb(dctcp_shift_g, &dctcp_shift_g_ops, &dctcp_shift_g, 0644); MODULE_PARM_DESC(dctcp_shift_g, "parameter g for updating dctcp_alpha"); static unsigned int dctcp_alpha_on_init __read_mostly = DCTCP_MAX_ALPHA; From 041933a1ec7b4173a8e638cae4f8e394331d7e54 Mon Sep 17 00:00:00 2001 From: Michal Luczaj Date: Fri, 17 May 2024 11:27:01 +0200 Subject: [PATCH 10/25] af_unix: Fix garbage collection of embryos carrying OOB with SCM_RIGHTS GC attempts to explicitly drop oob_skb's reference before purging the hit list. The problem is with embryos: kfree_skb(u->oob_skb) is never called on an embryo socket. The python script below [0] sends a listener's fd to its embryo as OOB data. While GC does collect the embryo's queue, it fails to drop the OOB skb's refcount. The skb which was in embryo's receive queue stays as unix_sk(sk)->oob_skb and keeps the listener's refcount [1]. Tell GC to dispose embryo's oob_skb. [0]: from array import array from socket import * addr = '\x00unix-oob' lis = socket(AF_UNIX, SOCK_STREAM) lis.bind(addr) lis.listen(1) s = socket(AF_UNIX, SOCK_STREAM) s.connect(addr) scm = (SOL_SOCKET, SCM_RIGHTS, array('i', [lis.fileno()])) s.sendmsg([b'x'], [scm], MSG_OOB) lis.close() [1] $ grep unix-oob /proc/net/unix $ ./unix-oob.py $ grep unix-oob /proc/net/unix 0000000000000000: 00000002 00000000 00000000 0001 02 0 @unix-oob 0000000000000000: 00000002 00000000 00010000 0001 01 6072 @unix-oob Fixes: 4090fa373f0e ("af_unix: Replace garbage collection algorithm.") Signed-off-by: Michal Luczaj Reviewed-by: Kuniyuki Iwashima Signed-off-by: Paolo Abeni --- net/unix/garbage.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/net/unix/garbage.c b/net/unix/garbage.c index 1f8b8cdfcdc8..dfe94a90ece4 100644 --- a/net/unix/garbage.c +++ b/net/unix/garbage.c @@ -342,6 +342,18 @@ enum unix_recv_queue_lock_class { U_RECVQ_LOCK_EMBRYO, }; +static void unix_collect_queue(struct unix_sock *u, struct sk_buff_head *hitlist) +{ + skb_queue_splice_init(&u->sk.sk_receive_queue, hitlist); + +#if IS_ENABLED(CONFIG_AF_UNIX_OOB) + if (u->oob_skb) { + WARN_ON_ONCE(skb_unref(u->oob_skb)); + u->oob_skb = NULL; + } +#endif +} + static void unix_collect_skb(struct list_head *scc, struct sk_buff_head *hitlist) { struct unix_vertex *vertex; @@ -365,18 +377,11 @@ static void unix_collect_skb(struct list_head *scc, struct sk_buff_head *hitlist /* listener -> embryo order, the inversion never happens. */ spin_lock_nested(&embryo_queue->lock, U_RECVQ_LOCK_EMBRYO); - skb_queue_splice_init(embryo_queue, hitlist); + unix_collect_queue(unix_sk(skb->sk), hitlist); spin_unlock(&embryo_queue->lock); } } else { - skb_queue_splice_init(queue, hitlist); - -#if IS_ENABLED(CONFIG_AF_UNIX_OOB) - if (u->oob_skb) { - kfree_skb(u->oob_skb); - u->oob_skb = NULL; - } -#endif + unix_collect_queue(u, hitlist); } spin_unlock(&queue->lock); From e060e433e51246d970c5a8aa1c5ccd9ecc7ba4bf Mon Sep 17 00:00:00 2001 From: Kuniyuki Iwashima Date: Fri, 17 May 2024 11:27:02 +0200 Subject: [PATCH 11/25] selftest: af_unix: Make SCM_RIGHTS into OOB data. scm_rights.c covers various test cases for inflight file descriptors and garbage collector for AF_UNIX sockets. Currently, SCM_RIGHTS messages are sent with 3-bytes string, and it's not good for MSG_OOB cases, as SCM_RIGTS cmsg goes with the first 2-bytes, which is non-OOB data. Let's send SCM_RIGHTS messages with 1-byte character to pack SCM_RIGHTS into OOB data. Signed-off-by: Kuniyuki Iwashima Signed-off-by: Michal Luczaj Signed-off-by: Paolo Abeni --- tools/testing/selftests/net/af_unix/scm_rights.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/net/af_unix/scm_rights.c b/tools/testing/selftests/net/af_unix/scm_rights.c index bab606c9f1eb..2bfed46e0b19 100644 --- a/tools/testing/selftests/net/af_unix/scm_rights.c +++ b/tools/testing/selftests/net/af_unix/scm_rights.c @@ -197,8 +197,8 @@ void __send_fd(struct __test_metadata *_metadata, const FIXTURE_VARIANT(scm_rights) *variant, int inflight, int receiver) { -#define MSG "nop" -#define MSGLEN 3 +#define MSG "x" +#define MSGLEN 1 struct { struct cmsghdr cmsghdr; int fd[2]; From 30a92c9e3d6b073932762bef2ac66f4ee784c657 Mon Sep 17 00:00:00 2001 From: Aaron Conole Date: Thu, 16 May 2024 16:09:41 -0400 Subject: [PATCH 12/25] openvswitch: Set the skbuff pkt_type for proper pmtud support. Open vSwitch is originally intended to switch at layer 2, only dealing with Ethernet frames. With the introduction of l3 tunnels support, it crossed into the realm of needing to care a bit about some routing details when making forwarding decisions. If an oversized packet would need to be fragmented during this forwarding decision, there is a chance for pmtu to get involved and generate a routing exception. This is gated by the skbuff->pkt_type field. When a flow is already loaded into the openvswitch module this field is set up and transitioned properly as a packet moves from one port to another. In the case that a packet execute is invoked after a flow is newly installed this field is not properly initialized. This causes the pmtud mechanism to omit sending the required exception messages across the tunnel boundary and a second attempt needs to be made to make sure that the routing exception is properly setup. To fix this, we set the outgoing packet's pkt_type to PACKET_OUTGOING, since it can only get to the openvswitch module via a port device or packet command. Even for bridge ports as users, the pkt_type needs to be reset when doing the transmit as the packet is truly outgoing and routing needs to get involved post packet transformations, in the case of VXLAN/GENEVE/udp-tunnel packets. In general, the pkt_type on output gets ignored, since we go straight to the driver, but in the case of tunnel ports they go through IP routing layer. This issue is periodically encountered in complex setups, such as large openshift deployments, where multiple sets of tunnel traversal occurs. A way to recreate this is with the ovn-heater project that can setup a networking environment which mimics such large deployments. We need larger environments for this because we need to ensure that flow misses occur. In these environment, without this patch, we can see: ./ovn_cluster.sh start podman exec ovn-chassis-1 ip r a 170.168.0.5/32 dev eth1 mtu 1200 podman exec ovn-chassis-1 ip netns exec sw01p1 ip r flush cache podman exec ovn-chassis-1 ip netns exec sw01p1 \ ping 21.0.0.3 -M do -s 1300 -c2 PING 21.0.0.3 (21.0.0.3) 1300(1328) bytes of data. From 21.0.0.3 icmp_seq=2 Frag needed and DF set (mtu = 1142) --- 21.0.0.3 ping statistics --- ... Using tcpdump, we can also see the expected ICMP FRAG_NEEDED message is not sent into the server. With this patch, setting the pkt_type, we see the following: podman exec ovn-chassis-1 ip netns exec sw01p1 \ ping 21.0.0.3 -M do -s 1300 -c2 PING 21.0.0.3 (21.0.0.3) 1300(1328) bytes of data. From 21.0.0.3 icmp_seq=1 Frag needed and DF set (mtu = 1222) ping: local error: message too long, mtu=1222 --- 21.0.0.3 ping statistics --- ... In this case, the first ping request receives the FRAG_NEEDED message and a local routing exception is created. Tested-by: Jaime Caamano Reported-at: https://issues.redhat.com/browse/FDP-164 Fixes: 58264848a5a7 ("openvswitch: Add vxlan tunneling support.") Signed-off-by: Aaron Conole Acked-by: Eelco Chaudron Link: https://lore.kernel.org/r/20240516200941.16152-1-aconole@redhat.com Signed-off-by: Paolo Abeni --- net/openvswitch/actions.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c index 6fcd7e2ca81f..964225580824 100644 --- a/net/openvswitch/actions.c +++ b/net/openvswitch/actions.c @@ -936,6 +936,12 @@ static void do_output(struct datapath *dp, struct sk_buff *skb, int out_port, pskb_trim(skb, ovs_mac_header_len(key)); } + /* Need to set the pkt_type to involve the routing layer. The + * packet movement through the OVS datapath doesn't generally + * use routing, but this is needed for tunnel cases. + */ + skb->pkt_type = PACKET_OUTGOING; + if (likely(!mru || (skb->len <= mru + vport->dev->hard_header_len))) { ovs_vport_send(vport, skb, ovs_key_mac_proto(key)); From eda40be3a5ff3fdce513d2bcfeaca8cc16cf962a Mon Sep 17 00:00:00 2001 From: Horatiu Vultur Date: Fri, 17 May 2024 15:58:08 +0200 Subject: [PATCH 13/25] net: lan966x: Remove ptp traps in case the ptp is not enabled. Lan966x is adding ptp traps to redirect the ptp frames to the CPU such that the HW will not forward these frames anywhere. The issue is that in case ptp is not enabled and the timestamping source is et to HWTSTAMP_SOURCE_NETDEV then these traps would not be removed on the error path. Fix this by removing the traps in this case as they are not needed. Fixes: 54e1ed69c40a ("net: lan966x: convert to ndo_hwtstamp_get() and ndo_hwtstamp_set()") Suggested-by: Vladimir Oltean Reviewed-by: Vladimir Oltean Signed-off-by: Horatiu Vultur Link: https://lore.kernel.org/r/20240517135808.3025435-1-horatiu.vultur@microchip.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/microchip/lan966x/lan966x_main.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c index a3b919a3ce07..ec672af12e25 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c @@ -474,14 +474,14 @@ static int lan966x_port_hwtstamp_set(struct net_device *dev, cfg->source != HWTSTAMP_SOURCE_PHYLIB) return -EOPNOTSUPP; + if (cfg->source == HWTSTAMP_SOURCE_NETDEV && !port->lan966x->ptp) + return -EOPNOTSUPP; + err = lan966x_ptp_setup_traps(port, cfg); if (err) return err; if (cfg->source == HWTSTAMP_SOURCE_NETDEV) { - if (!port->lan966x->ptp) - return -EOPNOTSUPP; - err = lan966x_ptp_hwtstamp_set(port, cfg, extack); if (err) { lan966x_ptp_del_traps(port); From 9c91c7fadb1771dcc2815c5271d14566366d05c5 Mon Sep 17 00:00:00 2001 From: Souradeep Chakrabarti Date: Sun, 19 May 2024 23:05:04 -0700 Subject: [PATCH 14/25] net: mana: Fix the extra HZ in mana_hwc_send_request Commit 62c1bff593b7 added an extra HZ along with msecs_to_jiffies. This patch fixes that. Cc: stable@vger.kernel.org Fixes: 62c1bff593b7 ("net: mana: Configure hwc timeout from hardware") Signed-off-by: Souradeep Chakrabarti Reviewed-by: Brett Creeley Reviewed-by: Dexuan Cui Link: https://lore.kernel.org/r/1716185104-31658-1-git-send-email-schakrabarti@linux.microsoft.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/microsoft/mana/hw_channel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c index 2729a2c5acf9..ca814fe8a775 100644 --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c @@ -848,7 +848,7 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len, } if (!wait_for_completion_timeout(&ctx->comp_event, - (msecs_to_jiffies(hwc->hwc_timeout) * HZ))) { + (msecs_to_jiffies(hwc->hwc_timeout)))) { dev_err(hwc->dev, "HWC: Request timed out!\n"); err = -ETIMEDOUT; goto out; From a61a459f58221f09810d6f60c657dda7add739fa Mon Sep 17 00:00:00 2001 From: Joe Damato Date: Mon, 20 May 2024 23:58:43 +0000 Subject: [PATCH 15/25] testing: net-drv: use stats64 for testing Testing a network device that has large numbers of bytes/packets may overflow. Using stats64 when comparing fixes this problem. I tripped on this while iterating on a qstats patch for mlx5. See below for confirmation without my added code that this is a bug. Before this patch (with added debugging output): $ NETIF=eth0 tools/testing/selftests/drivers/net/stats.py KTAP version 1 1..4 ok 1 stats.check_pause ok 2 stats.check_fec rstat: 481708634 qstat: 666201639514 key: tx-bytes not ok 3 stats.pkt_byte_sum ok 4 stats.qstat_by_ifindex Note the huge delta above ^^^ in the rtnl vs qstats. After this patch: $ NETIF=eth0 tools/testing/selftests/drivers/net/stats.py KTAP version 1 1..4 ok 1 stats.check_pause ok 2 stats.check_fec ok 3 stats.pkt_byte_sum ok 4 stats.qstat_by_ifindex It looks like rtnl_fill_stats in net/core/rtnetlink.c will attempt to copy the 64bit stats into a 32bit structure which is probably why this behavior is occurring. To show this is happening, you can get the underlying stats that the stats.py test uses like this: $ ./cli.py --spec ../../../Documentation/netlink/specs/rt_link.yaml \ --do getlink --json '{"ifi-index": 7}' And examine the output (heavily snipped to show relevant fields): 'stats': { 'multicast': 3739197, 'rx-bytes': 1201525399, 'rx-packets': 56807158, 'tx-bytes': 492404458, 'tx-packets': 1200285371, 'stats64': { 'multicast': 3739197, 'rx-bytes': 35561263767, 'rx-packets': 56807158, 'tx-bytes': 666212335338, 'tx-packets': 1200285371, The stats.py test prior to this patch was using the 'stats' structure above, which matches the failure output on my system. Comparing side by side, rx-bytes and tx-bytes, and getting ethtool -S output: rx-bytes stats: 1201525399 rx-bytes stats64: 35561263767 rx-bytes ethtool: 36203402638 tx-bytes stats: 492404458 tx-bytes stats64: 666212335338 tx-bytes ethtool: 666215360113 Note that the above was taken from a system with an mlx5 NIC, which only exposes ndo_get_stats64. Based on the ethtool output and qstat output, it appears that stats.py should be updated to use the 'stats64' structure for accurate comparisons when packet/byte counters get very large. To confirm that this was not related to the qstats code I was iterating on, I booted a kernel without my driver changes and re-ran the test which shows the qstats are skipped (as they don't exist for mlx5): NETIF=eth0 tools/testing/selftests/drivers/net/stats.py KTAP version 1 1..4 ok 1 stats.check_pause ok 2 stats.check_fec ok 3 stats.pkt_byte_sum # SKIP qstats not supported by the device ok 4 stats.qstat_by_ifindex # SKIP No ifindex supports qstats But, fetching the stats using the CLI $ ./cli.py --spec ../../../Documentation/netlink/specs/rt_link.yaml \ --do getlink --json '{"ifi-index": 7}' Shows the same issue (heavily snipped for relevant fields only): 'stats': { 'multicast': 105489, 'rx-bytes': 530879526, 'rx-packets': 751415, 'tx-bytes': 2510191396, 'tx-packets': 27700323, 'stats64': { 'multicast': 105489, 'rx-bytes': 530879526, 'rx-packets': 751415, 'tx-bytes': 15395093284, 'tx-packets': 27700323, Comparing side by side with ethtool -S on the unmodified mlx5 driver: tx-bytes stats: 2510191396 tx-bytes stats64: 15395093284 tx-bytes ethtool: 17718435810 Fixes: f0e6c86e4bab ("testing: net-drv: add a driver test for stats reporting") Signed-off-by: Joe Damato Link: https://lore.kernel.org/r/20240520235850.190041-1-jdamato@fastly.com Signed-off-by: Paolo Abeni --- tools/testing/selftests/drivers/net/stats.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/drivers/net/stats.py b/tools/testing/selftests/drivers/net/stats.py index 7a7b16b180e2..820b8e0a22c6 100755 --- a/tools/testing/selftests/drivers/net/stats.py +++ b/tools/testing/selftests/drivers/net/stats.py @@ -69,7 +69,7 @@ def pkt_byte_sum(cfg) -> None: return 0 for _ in range(10): - rtstat = rtnl.getlink({"ifi-index": cfg.ifindex})['stats'] + rtstat = rtnl.getlink({"ifi-index": cfg.ifindex})['stats64'] if stat_cmp(rtstat, qstat) < 0: raise Exception("RTNL stats are lower, fetched later") qstat = get_qstat(cfg) From b35b1c0b4e166a427395deaf61e3140495dfcb89 Mon Sep 17 00:00:00 2001 From: Jacob Keller Date: Mon, 20 May 2024 17:21:27 -0700 Subject: [PATCH 16/25] Revert "ixgbe: Manual AN-37 for troublesome link partners for X550 SFI" This reverts commit 565736048bd5f9888990569993c6b6bfdf6dcb6d. According to the commit, it implements a manual AN-37 for some "troublesome" Juniper MX5 switches. This appears to be a workaround for a particular switch. It has been reported that this causes a severe breakage for other switches, including a Cisco 3560CX-12PD-S. The code appears to be a workaround for a specific switch which fails to link in SFI mode. It expects to see AN-37 auto negotiation in order to link. The Cisco switch is not expecting AN-37 auto negotiation. When the device starts the manual AN-37, the Cisco switch decides that the port is confused and stops attempting to link with it. This persists until a power cycle. A simple driver unload and reload does not resolve the issue, even if loading with a version of the driver which lacks this workaround. The authors of the workaround commit have not responded with clarifications, and the result of the workaround is complete failure to connect with other switches. This appears to be a case where the driver can either "correctly" link with the Juniper MX5 switch, at the cost of bricking the link with the Cisco switch, or it can behave properly for the Cisco switch, but fail to link with the Junipir MX5 switch. I do not know enough about the standards involved to clearly determine whether either switch is at fault or behaving incorrectly. Nor do I know whether there exists some alternative fix which corrects behavior with both switches. Revert the workaround for the Juniper switch. Fixes: 565736048bd5 ("ixgbe: Manual AN-37 for troublesome link partners for X550 SFI") Link: https://lore.kernel.org/netdev/cbe874db-9ac9-42b8-afa0-88ea910e1e99@intel.com/T/ Link: https://forum.proxmox.com/threads/intel-x553-sfp-ixgbe-no-go-on-pve8.135129/#post-612291 Signed-off-by: Jacob Keller Cc: Jeff Daly Cc: kernel.org-fo5k2w@ycharbi.fr Reviewed-by: Simon Horman Link: https://lore.kernel.org/r/20240520-net-2024-05-20-revert-silicom-switch-workaround-v1-1-50f80f261c94@intel.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/intel/ixgbe/ixgbe_type.h | 3 - drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c | 56 +------------------ 2 files changed, 3 insertions(+), 56 deletions(-) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h index 897fe357b65b..346e3d9114a8 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_type.h @@ -3675,9 +3675,7 @@ struct ixgbe_info { #define IXGBE_KRM_LINK_S1(P) ((P) ? 0x8200 : 0x4200) #define IXGBE_KRM_LINK_CTRL_1(P) ((P) ? 0x820C : 0x420C) #define IXGBE_KRM_AN_CNTL_1(P) ((P) ? 0x822C : 0x422C) -#define IXGBE_KRM_AN_CNTL_4(P) ((P) ? 0x8238 : 0x4238) #define IXGBE_KRM_AN_CNTL_8(P) ((P) ? 0x8248 : 0x4248) -#define IXGBE_KRM_PCS_KX_AN(P) ((P) ? 0x9918 : 0x5918) #define IXGBE_KRM_SGMII_CTRL(P) ((P) ? 0x82A0 : 0x42A0) #define IXGBE_KRM_LP_BASE_PAGE_HIGH(P) ((P) ? 0x836C : 0x436C) #define IXGBE_KRM_DSP_TXFFE_STATE_4(P) ((P) ? 0x8634 : 0x4634) @@ -3687,7 +3685,6 @@ struct ixgbe_info { #define IXGBE_KRM_PMD_FLX_MASK_ST20(P) ((P) ? 0x9054 : 0x5054) #define IXGBE_KRM_TX_COEFF_CTRL_1(P) ((P) ? 0x9520 : 0x5520) #define IXGBE_KRM_RX_ANA_CTL(P) ((P) ? 0x9A00 : 0x5A00) -#define IXGBE_KRM_FLX_TMRS_CTRL_ST31(P) ((P) ? 0x9180 : 0x5180) #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_DA ~(0x3 << 20) #define IXGBE_KRM_PMD_FLX_MASK_ST20_SFI_10G_SR BIT(20) diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c index 2decb0710b6e..a5f644934445 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_x550.c @@ -1722,59 +1722,9 @@ static int ixgbe_setup_sfi_x550a(struct ixgbe_hw *hw, ixgbe_link_speed *speed) return -EINVAL; } - (void)mac->ops.write_iosf_sb_reg(hw, - IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); - - /* change mode enforcement rules to hybrid */ - (void)mac->ops.read_iosf_sb_reg(hw, - IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, ®_val); - reg_val |= 0x0400; - - (void)mac->ops.write_iosf_sb_reg(hw, - IXGBE_KRM_FLX_TMRS_CTRL_ST31(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); - - /* manually control the config */ - (void)mac->ops.read_iosf_sb_reg(hw, - IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, ®_val); - reg_val |= 0x20002240; - - (void)mac->ops.write_iosf_sb_reg(hw, - IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); - - /* move the AN base page values */ - (void)mac->ops.read_iosf_sb_reg(hw, - IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, ®_val); - reg_val |= 0x1; - - (void)mac->ops.write_iosf_sb_reg(hw, - IXGBE_KRM_PCS_KX_AN(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); - - /* set the AN37 over CB mode */ - (void)mac->ops.read_iosf_sb_reg(hw, - IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, ®_val); - reg_val |= 0x20000000; - - (void)mac->ops.write_iosf_sb_reg(hw, - IXGBE_KRM_AN_CNTL_4(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); - - /* restart AN manually */ - (void)mac->ops.read_iosf_sb_reg(hw, - IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, ®_val); - reg_val |= IXGBE_KRM_LINK_CTRL_1_TETH_AN_RESTART; - - (void)mac->ops.write_iosf_sb_reg(hw, - IXGBE_KRM_LINK_CTRL_1(hw->bus.lan_id), - IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); + status = mac->ops.write_iosf_sb_reg(hw, + IXGBE_KRM_PMD_FLX_MASK_ST20(hw->bus.lan_id), + IXGBE_SB_IOSF_TARGET_KR_PHY, reg_val); /* Toggle port SW reset by AN reset. */ status = ixgbe_restart_an_internal_phy_x550em(hw); From 3b1c92f8e5371700fada307cc8fd2c51fa7bc8c1 Mon Sep 17 00:00:00 2001 From: Wei Fang Date: Tue, 21 May 2024 10:38:00 +0800 Subject: [PATCH 17/25] net: fec: avoid lock evasion when reading pps_enable The assignment of pps_enable is protected by tmreg_lock, but the read operation of pps_enable is not. So the Coverity tool reports a lock evasion warning which may cause data race to occur when running in a multithread environment. Although this issue is almost impossible to occur, we'd better fix it, at least it seems more logically reasonable, and it also prevents Coverity from continuing to issue warnings. Fixes: 278d24047891 ("net: fec: ptp: Enable PPS output based on ptp clock") Signed-off-by: Wei Fang Link: https://lore.kernel.org/r/20240521023800.17102-1-wei.fang@nxp.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/freescale/fec_ptp.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/net/ethernet/freescale/fec_ptp.c b/drivers/net/ethernet/freescale/fec_ptp.c index 181d9bfbee22..e32f6724f568 100644 --- a/drivers/net/ethernet/freescale/fec_ptp.c +++ b/drivers/net/ethernet/freescale/fec_ptp.c @@ -104,14 +104,13 @@ static int fec_ptp_enable_pps(struct fec_enet_private *fep, uint enable) struct timespec64 ts; u64 ns; - if (fep->pps_enable == enable) - return 0; - - fep->pps_channel = DEFAULT_PPS_CHANNEL; - fep->reload_period = PPS_OUPUT_RELOAD_PERIOD; - spin_lock_irqsave(&fep->tmreg_lock, flags); + if (fep->pps_enable == enable) { + spin_unlock_irqrestore(&fep->tmreg_lock, flags); + return 0; + } + if (enable) { /* clear capture or output compare interrupt status if have. */ @@ -532,6 +531,9 @@ static int fec_ptp_enable(struct ptp_clock_info *ptp, int ret = 0; if (rq->type == PTP_CLK_REQ_PPS) { + fep->pps_channel = DEFAULT_PPS_CHANNEL; + fep->reload_period = PPS_OUPUT_RELOAD_PERIOD; + ret = fec_ptp_enable_pps(fep, on); return ret; From 91e61dd7a0af660408e87372d8330ceb218be302 Mon Sep 17 00:00:00 2001 From: "Dae R. Jeong" Date: Tue, 21 May 2024 19:34:38 +0900 Subject: [PATCH 18/25] tls: fix missing memory barrier in tls_init In tls_init(), a write memory barrier is missing, and store-store reordering may cause NULL dereference in tls_{setsockopt,getsockopt}. CPU0 CPU1 ----- ----- // In tls_init() // In tls_ctx_create() ctx = kzalloc() ctx->sk_proto = READ_ONCE(sk->sk_prot) -(1) // In update_sk_prot() WRITE_ONCE(sk->sk_prot, tls_prots) -(2) // In sock_common_setsockopt() READ_ONCE(sk->sk_prot)->setsockopt() // In tls_{setsockopt,getsockopt}() ctx->sk_proto->setsockopt() -(3) In the above scenario, when (1) and (2) are reordered, (3) can observe the NULL value of ctx->sk_proto, causing NULL dereference. To fix it, we rely on rcu_assign_pointer() which implies the release barrier semantic. By moving rcu_assign_pointer() after ctx->sk_proto is initialized, we can ensure that ctx->sk_proto are visible when changing sk->sk_prot. Fixes: d5bee7374b68 ("net/tls: Annotate access to sk_prot with READ_ONCE/WRITE_ONCE") Signed-off-by: Yewon Choi Signed-off-by: Dae R. Jeong Link: https://lore.kernel.org/netdev/ZU4OJG56g2V9z_H7@dragonet/T/ Link: https://lore.kernel.org/r/Zkx4vjSFp0mfpjQ2@libra05 Signed-off-by: Paolo Abeni --- net/tls/tls_main.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/tls/tls_main.c b/net/tls/tls_main.c index b4674f03d71a..90b7f253d363 100644 --- a/net/tls/tls_main.c +++ b/net/tls/tls_main.c @@ -816,9 +816,17 @@ struct tls_context *tls_ctx_create(struct sock *sk) return NULL; mutex_init(&ctx->tx_lock); - rcu_assign_pointer(icsk->icsk_ulp_data, ctx); ctx->sk_proto = READ_ONCE(sk->sk_prot); ctx->sk = sk; + /* Release semantic of rcu_assign_pointer() ensures that + * ctx->sk_proto is visible before changing sk->sk_prot in + * update_sk_prot(), and prevents reading uninitialized value in + * tls_{getsockopt, setsockopt}. Note that we do not need a + * read barrier in tls_{getsockopt,setsockopt} as there is an + * address dependency between sk->sk_proto->{getsockopt,setsockopt} + * and ctx->sk_proto. + */ + rcu_assign_pointer(icsk->icsk_ulp_data, ctx); return ctx; } From b31c7e78086127a7fcaa761e8d336ee855a920c6 Mon Sep 17 00:00:00 2001 From: Romain Gantois Date: Tue, 21 May 2024 14:44:11 +0200 Subject: [PATCH 19/25] net: ti: icssg_prueth: Fix NULL pointer dereference in prueth_probe() In the prueth_probe() function, if one of the calls to emac_phy_connect() fails due to of_phy_connect() returning NULL, then the subsequent call to phy_attached_info() will dereference a NULL pointer. Check the return code of emac_phy_connect and fail cleanly if there is an error. Fixes: 128d5874c082 ("net: ti: icssg-prueth: Add ICSSG ethernet driver") Cc: stable@vger.kernel.org Signed-off-by: Romain Gantois Reviewed-by: Simon Horman Reviewed-by: MD Danish Anwar Link: https://lore.kernel.org/r/20240521-icssg-prueth-fix-v1-1-b4b17b1433e9@bootlin.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/ti/icssg/icssg_prueth.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c index 7c9e9518f555..1ea3fbd5e954 100644 --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c @@ -1039,7 +1039,12 @@ static int prueth_probe(struct platform_device *pdev) prueth->registered_netdevs[PRUETH_MAC0] = prueth->emac[PRUETH_MAC0]->ndev; - emac_phy_connect(prueth->emac[PRUETH_MAC0]); + ret = emac_phy_connect(prueth->emac[PRUETH_MAC0]); + if (ret) { + dev_err(dev, + "can't connect to MII0 PHY, error -%d", ret); + goto netdev_unregister; + } phy_attached_info(prueth->emac[PRUETH_MAC0]->ndev->phydev); } @@ -1051,7 +1056,12 @@ static int prueth_probe(struct platform_device *pdev) } prueth->registered_netdevs[PRUETH_MAC1] = prueth->emac[PRUETH_MAC1]->ndev; - emac_phy_connect(prueth->emac[PRUETH_MAC1]); + ret = emac_phy_connect(prueth->emac[PRUETH_MAC1]); + if (ret) { + dev_err(dev, + "can't connect to MII1 PHY, error %d", ret); + goto netdev_unregister; + } phy_attached_info(prueth->emac[PRUETH_MAC1]->ndev->phydev); } From 378979e94e953c2070acb4f0e0c98d29260bd09d Mon Sep 17 00:00:00 2001 From: Jason Xing Date: Tue, 21 May 2024 21:42:20 +0800 Subject: [PATCH 20/25] tcp: remove 64 KByte limit for initial tp->rcv_wnd value Recently, we had some servers upgraded to the latest kernel and noticed the indicator from the user side showed worse results than before. It is caused by the limitation of tp->rcv_wnd. In 2018 commit a337531b942b ("tcp: up initial rmem to 128KB and SYN rwin to around 64KB") limited the initial value of tp->rcv_wnd to 65535, most CDN teams would not benefit from this change because they cannot have a large window to receive a big packet, which will be slowed down especially in long RTT. Small rcv_wnd means slow transfer speed, to some extent. It's the side effect for the latency/time-sensitive users. To avoid future confusion, current change doesn't affect the initial receive window on the wire in a SYN or SYN+ACK packet which are set within 65535 bytes according to RFC 7323 also due to the limit in __tcp_transmit_skb(): th->window = htons(min(tp->rcv_wnd, 65535U)); In one word, __tcp_transmit_skb() already ensures that constraint is respected, no matter how large tp->rcv_wnd is. The change doesn't violate RFC. Let me provide one example if with or without the patch: Before: client --- SYN: rwindow=65535 ---> server client <--- SYN+ACK: rwindow=65535 ---- server client --- ACK: rwindow=65536 ---> server Note: for the last ACK, the calculation is 512 << 7. After: client --- SYN: rwindow=65535 ---> server client <--- SYN+ACK: rwindow=65535 ---- server client --- ACK: rwindow=175232 ---> server Note: I use the following command to make it work: ip route change default via [ip] dev eth0 metric 100 initrwnd 120 For the last ACK, the calculation is 1369 << 7. When we apply such a patch, having a large rcv_wnd if the user tweak this knob can help transfer data more rapidly and save some rtts. Fixes: a337531b942b ("tcp: up initial rmem to 128KB and SYN rwin to around 64KB") Signed-off-by: Jason Xing Reviewed-by: Eric Dumazet Acked-by: Neal Cardwell Link: https://lore.kernel.org/r/20240521134220.12510-1-kerneljasonxing@gmail.com Signed-off-by: Paolo Abeni --- net/ipv4/tcp_output.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 95caf8aaa8be..95618d0e78e4 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -232,7 +232,7 @@ void tcp_select_initial_window(const struct sock *sk, int __space, __u32 mss, if (READ_ONCE(sock_net(sk)->ipv4.sysctl_tcp_workaround_signed_windows)) (*rcv_wnd) = min(space, MAX_TCP_WINDOW); else - (*rcv_wnd) = min_t(u32, space, U16_MAX); + (*rcv_wnd) = space; if (init_rcv_wnd) *rcv_wnd = min(*rcv_wnd, init_rcv_wnd * mss); From 26afda78cda3da974fd4c287962c169e9462c495 Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Tue, 21 May 2024 16:01:00 +0200 Subject: [PATCH 21/25] net: relax socket state check at accept time. Christoph reported the following splat: WARNING: CPU: 1 PID: 772 at net/ipv4/af_inet.c:761 __inet_accept+0x1f4/0x4a0 Modules linked in: CPU: 1 PID: 772 Comm: syz-executor510 Not tainted 6.9.0-rc7-g7da7119fe22b #56 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014 RIP: 0010:__inet_accept+0x1f4/0x4a0 net/ipv4/af_inet.c:759 Code: 04 38 84 c0 0f 85 87 00 00 00 41 c7 04 24 03 00 00 00 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc e8 ec b7 da fd <0f> 0b e9 7f fe ff ff e8 e0 b7 da fd 0f 0b e9 fe fe ff ff 89 d9 80 RSP: 0018:ffffc90000c2fc58 EFLAGS: 00010293 RAX: ffffffff836bdd14 RBX: 0000000000000000 RCX: ffff888104668000 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: dffffc0000000000 R08: ffffffff836bdb89 R09: fffff52000185f64 R10: dffffc0000000000 R11: fffff52000185f64 R12: dffffc0000000000 R13: 1ffff92000185f98 R14: ffff88810754d880 R15: ffff8881007b7800 FS: 000000001c772880(0000) GS:ffff88811b280000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007fb9fcf2e178 CR3: 00000001045d2002 CR4: 0000000000770ef0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 PKRU: 55555554 Call Trace: inet_accept+0x138/0x1d0 net/ipv4/af_inet.c:786 do_accept+0x435/0x620 net/socket.c:1929 __sys_accept4_file net/socket.c:1969 [inline] __sys_accept4+0x9b/0x110 net/socket.c:1999 __do_sys_accept net/socket.c:2016 [inline] __se_sys_accept net/socket.c:2013 [inline] __x64_sys_accept+0x7d/0x90 net/socket.c:2013 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0x58/0x100 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x76/0x7e RIP: 0033:0x4315f9 Code: fd ff 48 81 c4 80 00 00 00 e9 f1 fe ff ff 0f 1f 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 ab b4 fd ff c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007ffdb26d9c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002b RAX: ffffffffffffffda RBX: 0000000000400300 RCX: 00000000004315f9 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000004 RBP: 00000000006e1018 R08: 0000000000400300 R09: 0000000000400300 R10: 0000000000400300 R11: 0000000000000246 R12: 0000000000000000 R13: 000000000040cdf0 R14: 000000000040ce80 R15: 0000000000000055 The reproducer invokes shutdown() before entering the listener status. After commit 94062790aedb ("tcp: defer shutdown(SEND_SHUTDOWN) for TCP_SYN_RECV sockets"), the above causes the child to reach the accept syscall in FIN_WAIT1 status. Eric noted we can relax the existing assertion in __inet_accept() Reported-by: Christoph Paasch Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/490 Suggested-by: Eric Dumazet Fixes: 94062790aedb ("tcp: defer shutdown(SEND_SHUTDOWN) for TCP_SYN_RECV sockets") Reviewed-by: Eric Dumazet Link: https://lore.kernel.org/r/23ab880a44d8cfd967e84de8b93dbf48848e3d8c.1716299669.git.pabeni@redhat.com Signed-off-by: Paolo Abeni --- net/ipv4/af_inet.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c index 44564d009e95..86cce7e9c2d1 100644 --- a/net/ipv4/af_inet.c +++ b/net/ipv4/af_inet.c @@ -758,7 +758,9 @@ void __inet_accept(struct socket *sock, struct socket *newsock, struct sock *new sock_rps_record_flow(newsk); WARN_ON(!((1 << newsk->sk_state) & (TCPF_ESTABLISHED | TCPF_SYN_RECV | - TCPF_CLOSE_WAIT | TCPF_CLOSE))); + TCPF_FIN_WAIT1 | TCPF_FIN_WAIT2 | + TCPF_CLOSING | TCPF_CLOSE_WAIT | + TCPF_CLOSE))); if (test_bit(SOCK_SUPPORT_ZC, &sock->flags)) set_bit(SOCK_SUPPORT_ZC, &newsock->flags); From 6671e352497ca4bb07a96c48e03907065ff77d8a Mon Sep 17 00:00:00 2001 From: Ryosuke Yasuoka Date: Wed, 22 May 2024 00:34:42 +0900 Subject: [PATCH 22/25] nfc: nci: Fix handling of zero-length payload packets in nci_rx_work() When nci_rx_work() receives a zero-length payload packet, it should not discard the packet and exit the loop. Instead, it should continue processing subsequent packets. Fixes: d24b03535e5e ("nfc: nci: Fix uninit-value in nci_dev_up and nci_ntf_packet") Signed-off-by: Ryosuke Yasuoka Reviewed-by: Simon Horman Reviewed-by: Krzysztof Kozlowski Link: https://lore.kernel.org/r/20240521153444.535399-1-ryasuoka@redhat.com Signed-off-by: Paolo Abeni --- net/nfc/nci/core.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c index 7a9897fbf4f4..f456a5911e7d 100644 --- a/net/nfc/nci/core.c +++ b/net/nfc/nci/core.c @@ -1531,8 +1531,7 @@ static void nci_rx_work(struct work_struct *work) if (!nci_valid_size(skb)) { kfree_skb(skb); - kcov_remote_stop(); - break; + continue; } /* Process frame */ From 05d6f442f31f901d27dbc64fd504a8ec7d5013de Mon Sep 17 00:00:00 2001 From: Larysa Zaremba Date: Tue, 21 May 2024 12:39:53 -0700 Subject: [PATCH 23/25] ice: Interpret .set_channels() input differently A bug occurs because a safety check guarding AF_XDP-related queues in ethnl_set_channels(), does not trigger. This happens, because kernel and ice driver interpret the ethtool command differently. How the bug occurs: 1. ethtool -l -> combined: 40 2. Attach AF_XDP to queue 30 3. ethtool -L rx 15 tx 15 combined number is not specified, so command becomes {rx_count = 15, tx_count = 15, combined_count = 40}. 4. ethnl_set_channels checks, if there are any AF_XDP of queues from the new (combined_count + rx_count) to the old one, so from 55 to 40, check does not trigger. 5. ice interprets `rx 15 tx 15` as 15 combined channels and deletes the queue that AF_XDP is attached to. Interpret the command in a way that is more consistent with ethtool manual [0] (--show-channels and --set-channels). Considering that in the ice driver only the difference between RX and TX queues forms dedicated channels, change the correct way to set number of channels to: ethtool -L combined 10 /* For symmetric queues */ ethtool -L combined 8 tx 2 rx 0 /* For asymmetric queues */ [0] https://man7.org/linux/man-pages/man8/ethtool.8.html Fixes: 87324e747fde ("ice: Implement ethtool ops for channels") Reviewed-by: Michal Swiatkowski Signed-off-by: Larysa Zaremba Tested-by: Chandan Kumar Rout Tested-by: Pucha Himasekhar Reddy Acked-by: Maciej Fijalkowski Signed-off-by: Jacob Keller Signed-off-by: Paolo Abeni --- drivers/net/ethernet/intel/ice/ice_ethtool.c | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c index 78b833b3e1d7..62c8205fceba 100644 --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c @@ -3593,7 +3593,6 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch) struct ice_pf *pf = vsi->back; int new_rx = 0, new_tx = 0; bool locked = false; - u32 curr_combined; int ret = 0; /* do not support changing channels in Safe Mode */ @@ -3615,22 +3614,8 @@ static int ice_set_channels(struct net_device *dev, struct ethtool_channels *ch) return -EOPNOTSUPP; } - curr_combined = ice_get_combined_cnt(vsi); - - /* these checks are for cases where user didn't specify a particular - * value on cmd line but we get non-zero value anyway via - * get_channels(); look at ethtool.c in ethtool repository (the user - * space part), particularly, do_schannels() routine - */ - if (ch->rx_count == vsi->num_rxq - curr_combined) - ch->rx_count = 0; - if (ch->tx_count == vsi->num_txq - curr_combined) - ch->tx_count = 0; - if (ch->combined_count == curr_combined) - ch->combined_count = 0; - - if (!(ch->combined_count || (ch->rx_count && ch->tx_count))) { - netdev_err(dev, "Please specify at least 1 Rx and 1 Tx channel\n"); + if (ch->rx_count && ch->tx_count) { + netdev_err(dev, "Dedicated RX or TX channels cannot be used simultaneously\n"); return -EINVAL; } From 5e7695e0219bf6acb96081af3ba0ca08b1829656 Mon Sep 17 00:00:00 2001 From: Larysa Zaremba Date: Tue, 21 May 2024 12:39:54 -0700 Subject: [PATCH 24/25] idpf: Interpret .set_channels() input differently Unlike ice, idpf does not check, if user has requested at least 1 combined channel. Instead, it relies on a check in the core code. Unfortunately, the check does not trigger for us because of the hacky .set_channels() interpretation logic that is not consistent with the core code. This naturally leads to user being able to trigger a crash with an invalid input. This is how: 1. ethtool -l -> combined: 40 2. ethtool -L rx 0 tx 0 combined number is not specified, so command becomes {rx_count = 0, tx_count = 0, combined_count = 40}. 3. ethnl_set_channels checks, if there is at least 1 RX and 1 TX channel, comparing (combined_count + rx_count) and (combined_count + tx_count) to zero. Obviously, (40 + 0) is greater than zero, so the core code deems the input OK. 4. idpf interprets `rx 0 tx 0` as 0 channels and tries to proceed with such configuration. The issue has to be solved fundamentally, as current logic is also known to cause AF_XDP problems in ice [0]. Interpret the command in a way that is more consistent with ethtool manual [1] (--show-channels and --set-channels) and new ice logic. Considering that in the idpf driver only the difference between RX and TX queues forms dedicated channels, change the correct way to set number of channels to: ethtool -L combined 10 /* For symmetric queues */ ethtool -L combined 8 tx 2 rx 0 /* For asymmetric queues */ [0] https://lore.kernel.org/netdev/20240418095857.2827-1-larysa.zaremba@intel.com/ [1] https://man7.org/linux/man-pages/man8/ethtool.8.html Fixes: 02cbfba1add5 ("idpf: add ethtool callbacks") Reviewed-by: Przemek Kitszel Reviewed-by: Igor Bagnucki Signed-off-by: Larysa Zaremba Tested-by: Krishneil Singh Reviewed-by: Simon Horman Signed-off-by: Jacob Keller Signed-off-by: Paolo Abeni --- .../net/ethernet/intel/idpf/idpf_ethtool.c | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c index 6972d728431c..1885ba618981 100644 --- a/drivers/net/ethernet/intel/idpf/idpf_ethtool.c +++ b/drivers/net/ethernet/intel/idpf/idpf_ethtool.c @@ -222,14 +222,19 @@ static int idpf_set_channels(struct net_device *netdev, struct ethtool_channels *ch) { struct idpf_vport_config *vport_config; - u16 combined, num_txq, num_rxq; unsigned int num_req_tx_q; unsigned int num_req_rx_q; struct idpf_vport *vport; + u16 num_txq, num_rxq; struct device *dev; int err = 0; u16 idx; + if (ch->rx_count && ch->tx_count) { + netdev_err(netdev, "Dedicated RX or TX channels cannot be used simultaneously\n"); + return -EINVAL; + } + idpf_vport_ctrl_lock(netdev); vport = idpf_netdev_to_vport(netdev); @@ -239,20 +244,6 @@ static int idpf_set_channels(struct net_device *netdev, num_txq = vport_config->user_config.num_req_tx_qs; num_rxq = vport_config->user_config.num_req_rx_qs; - combined = min(num_txq, num_rxq); - - /* these checks are for cases where user didn't specify a particular - * value on cmd line but we get non-zero value anyway via - * get_channels(); look at ethtool.c in ethtool repository (the user - * space part), particularly, do_schannels() routine - */ - if (ch->combined_count == combined) - ch->combined_count = 0; - if (ch->combined_count && ch->rx_count == num_rxq - combined) - ch->rx_count = 0; - if (ch->combined_count && ch->tx_count == num_txq - combined) - ch->tx_count = 0; - num_req_tx_q = ch->combined_count + ch->tx_count; num_req_rx_q = ch->combined_count + ch->rx_count; From c71e3a5cffd5309d7f84444df03d5b72600cc417 Mon Sep 17 00:00:00 2001 From: Ken Milmore Date: Tue, 21 May 2024 23:45:50 +0100 Subject: [PATCH 25/25] r8169: Fix possible ring buffer corruption on fragmented Tx packets. An issue was found on the RTL8125b when transmitting small fragmented packets, whereby invalid entries were inserted into the transmit ring buffer, subsequently leading to calls to dma_unmap_single() with a null address. This was caused by rtl8169_start_xmit() not noticing changes to nr_frags which may occur when small packets are padded (to work around hardware quirks) in rtl8169_tso_csum_v2(). To fix this, postpone inspecting nr_frags until after any padding has been applied. Fixes: 9020845fb5d6 ("r8169: improve rtl8169_start_xmit") Cc: stable@vger.kernel.org Signed-off-by: Ken Milmore Reviewed-by: Heiner Kallweit Link: https://lore.kernel.org/r/27ead18b-c23d-4f49-a020-1fc482c5ac95@gmail.com Signed-off-by: Paolo Abeni --- drivers/net/ethernet/realtek/r8169_main.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 86a6d4225bc2..9b0ef00b99d6 100644 --- a/drivers/net/ethernet/realtek/r8169_main.c +++ b/drivers/net/ethernet/realtek/r8169_main.c @@ -4337,11 +4337,11 @@ static void rtl8169_doorbell(struct rtl8169_private *tp) static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb, struct net_device *dev) { - unsigned int frags = skb_shinfo(skb)->nr_frags; struct rtl8169_private *tp = netdev_priv(dev); unsigned int entry = tp->cur_tx % NUM_TX_DESC; struct TxDesc *txd_first, *txd_last; bool stop_queue, door_bell; + unsigned int frags; u32 opts[2]; if (unlikely(!rtl_tx_slots_avail(tp))) { @@ -4364,6 +4364,7 @@ static netdev_tx_t rtl8169_start_xmit(struct sk_buff *skb, txd_first = tp->TxDescArray + entry; + frags = skb_shinfo(skb)->nr_frags; if (frags) { if (rtl8169_xmit_frags(tp, skb, opts, entry)) goto err_dma_1;