From 712a6f545c359b427daa9a5a782e30d2f8331e25 Mon Sep 17 00:00:00 2001 From: Jozsef Kadlecsik Date: Thu, 30 Jul 2026 20:38:50 +0200 Subject: netfilter: ipset: switch ext_size to atomic64_t The hash types do not acquire set->lock, they use 'region locking' where only part of the hash table is locked. Parallel inserts and deletes are possible and CPUs can race on ->ext_size update. Switch to atomic64_t. This leaves another bug unresolved: there still can be a race on comment extension re-init. This will be handled in a later commit when converting to rhashtable backend. Fixes: f66ee0410b1c ("netfilter: ipset: Fix "INFO: rcu detected stall in hash_xxx" reports") Signed-off-by: Jozsef Kadlecsik Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter/ipset/ip_set.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h index b98331572ad2..cadae9b2578f 100644 --- a/include/linux/netfilter/ipset/ip_set.h +++ b/include/linux/netfilter/ipset/ip_set.h @@ -273,7 +273,7 @@ struct ip_set { /* Number of elements (vs timeout) */ u32 elements; /* Size of the dynamic extensions (vs timeout) */ - size_t ext_size; + atomic64_t ext_size; /* Element data size */ size_t dsize; /* Offsets to extensions in elements */ -- cgit From 7defddefa95b3699b05b23bf3ceea94d872509bb Mon Sep 17 00:00:00 2001 From: Florian Westphal Date: Thu, 30 Jul 2026 20:38:53 +0200 Subject: netfilter: ipset: switch to rcu work In the initial ipset rhashtable conversion RFC series syzbot reported following splat: BUG: sleeping function [..] at kernel/irq_work.c:289 in_atomic(): 1, [..] irq_work_sync.. kernel/irq_work.c:289 rhashtable_free_and_destroy.. lib/rhashtable.c:1295 hash_netport4_destroy.. net/netfilter/ipset/ip_set_hash_gen.h:420 ip_set_destroy_set_rcu.. net/netfilter/ipset/ip_set_core.c:1169 rcu_core.. kernel/rcu/tree.c:2897 This is because post-rhashtable-conversion hash implementation needs to schedule in the destroy callback. At this time this isn't allowed. Replace existing call_rcu() based destruction with rcu_work api. Also allows to undo split of set destruction and gc work cancelling in a future patch. Signed-off-by: Florian Westphal Signed-off-by: Pablo Neira Ayuso --- include/linux/netfilter/ipset/ip_set.h | 4 +-- net/netfilter/ipset/ip_set_core.c | 46 +++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 19 deletions(-) (limited to 'include') diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h index cadae9b2578f..c46864cc6623 100644 --- a/include/linux/netfilter/ipset/ip_set.h +++ b/include/linux/netfilter/ipset/ip_set.h @@ -244,8 +244,8 @@ extern void ip_set_type_unregister(struct ip_set_type *set_type); /* A generic IP set */ struct ip_set { - /* For call_cru in destroy */ - struct rcu_head rcu; + /* for set destruction */ + struct rcu_work rwork; /* The name of the set */ char name[IPSET_MAXNAMELEN]; /* Lock protecting the set data */ diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c index 822a53a7f502..543851a923d0 100644 --- a/net/netfilter/ipset/ip_set_core.c +++ b/net/netfilter/ipset/ip_set_core.c @@ -25,6 +25,7 @@ static LIST_HEAD(ip_set_type_list); /* all registered set types */ static DEFINE_MUTEX(ip_set_type_mutex); /* protects ip_set_type_list */ static DEFINE_RWLOCK(ip_set_ref_lock); /* protects the set refs */ +static struct workqueue_struct *ipset_destroy_wq; struct ip_set_net { struct ip_set * __rcu *ip_set_list; /* all individual sets */ @@ -1178,22 +1179,26 @@ ip_set_setname_policy[IPSET_ATTR_CMD_MAX + 1] = { .len = IPSET_MAXNAMELEN - 1 }, }; -/* In order to return quickly when destroying a single set, it is split - * into two stages: - * - Cancel garbage collector - * - Destroy the set itself via call_rcu() - */ - static void -ip_set_destroy_set_rcu(struct rcu_head *head) +destroy_and_free_set(struct ip_set *set) { - struct ip_set *set = container_of(head, struct ip_set, rcu); - set->variant->destroy(set); module_put(set->type->me); kfree(set); } +/* In order to return quickly when destroying a single set, + * destruction is done asynchronously via work queues. + */ +static void +ip_set_destroy_set_work(struct work_struct *work) +{ + struct ip_set *set = container_of(to_rcu_work(work), + struct ip_set, rwork); + + destroy_and_free_set(set); +} + static void _destroy_all_sets(struct ip_set_net *inst) { @@ -1283,7 +1288,8 @@ static int ip_set_destroy(struct sk_buff *skb, const struct nfnl_info *info, /* Must wait for flush to be really finished */ rcu_barrier(); } - call_rcu(&s->rcu, ip_set_destroy_set_rcu); + INIT_RCU_WORK(&s->rwork, ip_set_destroy_set_work); + queue_rcu_work(ipset_destroy_wq, &s->rwork); } return 0; out: @@ -2421,18 +2427,23 @@ static struct pernet_operations ip_set_net_ops = { static int __init ip_set_init(void) { - int ret = register_pernet_subsys(&ip_set_net_ops); + int ret; + + ipset_destroy_wq = alloc_ordered_workqueue("ipset_destroy_wq", 0); + if (!ipset_destroy_wq) + return -ENOMEM; + ret = register_pernet_subsys(&ip_set_net_ops); if (ret) { pr_err("ip_set: cannot register pernet_subsys.\n"); - return ret; + goto out_wq; } ret = nfnetlink_subsys_register(&ip_set_netlink_subsys); if (ret != 0) { pr_err("ip_set: cannot register with nfnetlink.\n"); unregister_pernet_subsys(&ip_set_net_ops); - return ret; + goto out_wq; } ret = nf_register_sockopt(&so_set); @@ -2440,10 +2451,13 @@ ip_set_init(void) pr_err("SO_SET registry failed: %d\n", ret); nfnetlink_subsys_unregister(&ip_set_netlink_subsys); unregister_pernet_subsys(&ip_set_net_ops); - return ret; + goto out_wq; } return 0; +out_wq: + destroy_workqueue(ipset_destroy_wq); + return ret; } static void __exit @@ -2453,9 +2467,7 @@ ip_set_fini(void) nfnetlink_subsys_unregister(&ip_set_netlink_subsys); unregister_pernet_subsys(&ip_set_net_ops); - /* Wait for call_rcu() in destroy */ - rcu_barrier(); - + destroy_workqueue(ipset_destroy_wq); pr_debug("these are the famous last words\n"); } -- cgit From 646922a0379496154e8c8faca4f8e2fd9100cacc Mon Sep 17 00:00:00 2001 From: Julian Anastasov Date: Thu, 30 Jul 2026 21:35:05 +0300 Subject: ipvs: avoid out-of-bounds write in ip_vs_nat_icmp Sashiko warns that local attacker can modify the packet while it is processed by IPVS. Some places read the IP ihl field multiple times which can cause out-of-bounds access. One such place is ip_vs_nat_icmp where we can write after the validated area. Fix it by providing ciph argument just like it is done for IPv6 and use ciph->len as offset to the embedded transport header. Modify some IPv4 header checks by reading the ihl field only once. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Link: https://sashiko.dev/#/patchset/20260722101517.36313-1-ja%40ssi.bg Signed-off-by: Julian Anastasov Signed-off-by: Pablo Neira Ayuso --- include/net/ip_vs.h | 2 +- net/netfilter/ipvs/ip_vs_core.c | 67 +++++++++++++++++++++-------------------- net/netfilter/ipvs/ip_vs_xmit.c | 2 +- 3 files changed, 36 insertions(+), 35 deletions(-) (limited to 'include') diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index e6ca930a3507..1235f1934e94 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -2062,7 +2062,7 @@ static inline bool ip_vs_conn_use_hash2(struct ip_vs_conn *cp) void ip_vs_nat_icmp(struct sk_buff *skb, struct ip_vs_protocol *pp, struct ip_vs_conn *cp, int dir, unsigned int toff, - bool has_ports); + bool has_ports, struct ip_vs_iphdr *ciph); #ifdef CONFIG_IP_VS_IPV6 void ip_vs_nat_icmp_v6(struct sk_buff *skb, struct ip_vs_protocol *pp, diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c index 6b79e0c4d9e2..0bdaeb4ed61e 100644 --- a/net/netfilter/ipvs/ip_vs_core.c +++ b/net/netfilter/ipvs/ip_vs_core.c @@ -925,28 +925,27 @@ static int ip_vs_route_me_harder(struct netns_ipvs *ipvs, int af, */ void ip_vs_nat_icmp(struct sk_buff *skb, struct ip_vs_protocol *pp, struct ip_vs_conn *cp, int inout, unsigned int toff, - bool has_ports) + bool has_ports, struct ip_vs_iphdr *ciph) { struct iphdr *iph = ip_hdr(skb); struct icmphdr *icmph = (struct icmphdr *)(skb->data + toff); - struct iphdr *ciph = (struct iphdr *)(icmph + 1); - unsigned int coff __maybe_unused = toff + sizeof(struct icmphdr); + struct iphdr *cih = (struct iphdr *)(icmph + 1); if (inout) { iph->saddr = cp->vaddr.ip; ip_send_check(iph); - ciph->daddr = cp->vaddr.ip; - ip_send_check(ciph); + cih->daddr = cp->vaddr.ip; + ip_send_check(cih); } else { iph->daddr = cp->daddr.ip; ip_send_check(iph); - ciph->saddr = cp->daddr.ip; - ip_send_check(ciph); + cih->saddr = cp->daddr.ip; + ip_send_check(cih); } /* the TCP/UDP/SCTP port */ if (has_ports) { - __be16 *ports = (void *)ciph + ciph->ihl*4; + __be16 *ports = (void *)(skb->data + ciph->len); if (inout) ports[1] = cp->vport; @@ -960,10 +959,10 @@ void ip_vs_nat_icmp(struct sk_buff *skb, struct ip_vs_protocol *pp, skb->ip_summed = CHECKSUM_UNNECESSARY; if (inout) - IP_VS_DBG_PKT(11, AF_INET, pp, skb, coff, + IP_VS_DBG_PKT(11, AF_INET, pp, skb, ciph->off, "Forwarding altered outgoing ICMP"); else - IP_VS_DBG_PKT(11, AF_INET, pp, skb, coff, + IP_VS_DBG_PKT(11, AF_INET, pp, skb, ciph->off, "Forwarding altered incoming ICMP"); } @@ -1056,7 +1055,7 @@ static int handle_response_icmp(int af, struct sk_buff *skb, ip_vs_nat_icmp_v6(skb, pp, cp, 1, toff, has_ports, ciph); else #endif - ip_vs_nat_icmp(skb, pp, cp, 1, toff, has_ports); + ip_vs_nat_icmp(skb, pp, cp, 1, toff, has_ports, ciph); if (ip_vs_route_me_harder(cp->ipvs, af, skb, hooknum)) goto out; @@ -1092,7 +1091,7 @@ static int ip_vs_out_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, struct ip_vs_iphdr ciph; struct ip_vs_conn *cp; struct ip_vs_protocol *pp; - unsigned int offset, ihl; + unsigned int offset; union nf_inet_addr snet; *related = 1; @@ -1105,7 +1104,6 @@ static int ip_vs_out_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, return NF_ACCEPT; } - ihl = ipvsh->len; offset = ipvsh->len; ic = skb_header_pointer(skb, offset, sizeof(_icmph), &_icmph); if (ic == NULL) @@ -1131,11 +1129,15 @@ static int ip_vs_out_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, /* Now find the contained IP header */ offset += sizeof(_icmph); + if (!ip_vs_fill_iph_skb_icmp(AF_INET, skb, offset, true, &ciph)) + return NF_ACCEPT; /* The packet looks wrong, ignore */ + cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); - if (!(cih && cih->version == 4 && cih->ihl >= 5)) + if (!(cih && cih->version == 4 && + ciph.len - ciph.off >= sizeof(struct iphdr))) return NF_ACCEPT; /* The packet looks wrong, ignore */ - pp = ip_vs_proto_get(cih->protocol); + pp = ip_vs_proto_get(ciph.protocol); if (!pp) return NF_ACCEPT; @@ -1146,8 +1148,6 @@ static int ip_vs_out_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, IP_VS_DBG_PKT(11, AF_INET, pp, skb, offset, "Checking outgoing ICMP for"); - ip_vs_fill_iph_skb_icmp(AF_INET, skb, offset, true, &ciph); - /* The embedded headers contain source and dest in reverse order */ cp = INDIRECT_CALL_1(pp->conn_out_get, ip_vs_conn_out_get_proto, ipvs, AF_INET, skb, &ciph); @@ -1155,8 +1155,8 @@ static int ip_vs_out_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, return NF_ACCEPT; snet.ip = ipvsh->saddr.ip; - return handle_response_icmp(AF_INET, skb, &snet, cp, pp, &ciph, ihl, - hooknum); + return handle_response_icmp(AF_INET, skb, &snet, cp, pp, &ciph, + ipvsh->len, hooknum); } #ifdef CONFIG_IP_VS_IPV6 @@ -1803,10 +1803,12 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, /* Now find the contained IP header */ offset += sizeof(_icmph); cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); - if (!(cih && cih->version == 4 && cih->ihl >= 5)) + if (!cih) return NF_ACCEPT; /* The packet looks wrong, ignore */ - raddr = (union nf_inet_addr *)&cih->daddr; hlen_ipip = cih->ihl * 4; + if (!(cih->version == 4 && hlen_ipip >= sizeof(struct iphdr))) + return NF_ACCEPT; /* The packet looks wrong, ignore */ + raddr = (union nf_inet_addr *)&cih->daddr; /* Special case for errors for IPIP/UDP/GRE tunnel packets */ tunnel = false; @@ -1823,9 +1825,6 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, if (!dest || dest->tun_type != IP_VS_CONN_F_TUNNEL_TYPE_IPIP) return NF_ACCEPT; offset += hlen_ipip; - cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); - if (!(cih && cih->version == 4 && cih->ihl >= 5)) - return NF_ACCEPT; /* The packet looks wrong, ignore */ tunnel = true; } else if ((cih->protocol == IPPROTO_UDP || /* Can be UDP encap */ cih->protocol == IPPROTO_GRE) && /* Can be GRE encap */ @@ -1850,21 +1849,25 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, /* Skip IP and UDP/GRE tunnel headers */ offset = offset2 + ulen; /* Now we should be at the original IP header */ - cih = skb_header_pointer(skb, offset, sizeof(_ciph), - &_ciph); - if (cih && cih->version == 4 && cih->ihl >= 5 && - iproto == IPPROTO_IPIP) + if (iproto == IPPROTO_IPIP) tunnel = true; else return NF_ACCEPT; } } - pd = ip_vs_proto_data_get(ipvs, cih->protocol); + if (!ip_vs_fill_iph_skb_icmp(AF_INET, skb, offset, !tunnel, &ciph)) + return NF_ACCEPT; + pd = ip_vs_proto_data_get(ipvs, ciph.protocol); if (!pd) return NF_ACCEPT; pp = pd->pp; + cih = skb_header_pointer(skb, offset, sizeof(_ciph), &_ciph); + if (!(cih && cih->version == 4 && + ciph.len - ciph.off >= sizeof(struct iphdr))) + return NF_ACCEPT; /* The packet looks wrong, ignore */ + /* Is the embedded protocol header present? */ if (unlikely(cih->frag_off & htons(IP_OFFSET) && !pp->dont_defrag)) return NF_ACCEPT; @@ -1872,9 +1875,6 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, IP_VS_DBG_PKT(11, AF_INET, pp, skb, offset, "Checking incoming ICMP for"); - offset2 = offset; - ip_vs_fill_iph_skb_icmp(AF_INET, skb, offset, !tunnel, &ciph); - /* The embedded headers contain source and dest in reverse order. * For IPIP/UDP/GRE tunnel this is error for request, not for reply. */ @@ -1904,11 +1904,12 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related, } if (tunnel) { - unsigned int hlen_orig = cih->ihl * 4; + unsigned int hlen_orig = ciph.len - ciph.off; __be32 info = ic->un.gateway; __u8 type = ic->type; __u8 code = ic->code; + offset2 = offset; /* Update the MTU */ if (ic->type == ICMP_DEST_UNREACH && ic->code == ICMP_FRAG_NEEDED) { diff --git a/net/netfilter/ipvs/ip_vs_xmit.c b/net/netfilter/ipvs/ip_vs_xmit.c index 0b0c5304993a..c4508f3f43dd 100644 --- a/net/netfilter/ipvs/ip_vs_xmit.c +++ b/net/netfilter/ipvs/ip_vs_xmit.c @@ -1580,7 +1580,7 @@ ip_vs_icmp_xmit(struct sk_buff *skb, struct ip_vs_conn *cp, if (skb_cow(skb, rt->dst.dev->hard_header_len)) goto tx_error; - ip_vs_nat_icmp(skb, pp, cp, 0, toff, has_ports); + ip_vs_nat_icmp(skb, pp, cp, 0, toff, has_ports, ciph); /* Another hack: avoid icmp_send in ip_fragment */ skb->ignore_df = 1; -- cgit From 99609cb0aa789c8d071050ce8579989551882cc6 Mon Sep 17 00:00:00 2001 From: Julian Anastasov Date: Thu, 30 Jul 2026 21:35:06 +0300 Subject: ipvs: return the csum validation for forward hook Sashiko notes that playing games with the skb dst and rt flags instead of providing hooknum is not a good idea when validating the checksums. Also, skipping checksum validation for FORWARD packets risk silent data corruption, even if the only user is the FTP-CMD packets coming from the real server. Sashiko also noticed that by using common checksum helper in the previous commit we actually fixed old bug where the TCP/UDP checksum for IPv6 on CHECKSUM_COMPLETE was not validated correctly. Fixes: e876b75b9020 ("ipvs: fix the checksum validations") Link: https://sashiko.dev/#/patchset/20260722211420.153933-1-pablo%40netfilter.org Link: https://sashiko.dev/#/patchset/20260727185024.67534-1-ja%40ssi.bg Link: https://sashiko.dev/#/patchset/20260728202520.59179-1-ja%40ssi.bg Signed-off-by: Julian Anastasov Signed-off-by: Pablo Neira Ayuso --- include/net/ip_vs.h | 19 +++++-------------- net/netfilter/ipvs/ip_vs_proto_sctp.c | 2 +- 2 files changed, 6 insertions(+), 15 deletions(-) (limited to 'include') diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h index 1235f1934e94..d2813eb795be 100644 --- a/include/net/ip_vs.h +++ b/include/net/ip_vs.h @@ -25,9 +25,7 @@ #include /* for union nf_inet_addr */ #include #include /* for struct ipv6hdr */ -#include #include -#include #if IS_ENABLED(CONFIG_NF_CONNTRACK) #include #endif @@ -2095,30 +2093,23 @@ static inline __wsum ip_vs_check_diff2(__be16 old, __be16 new, __wsum oldsum) return csum_partial(diff, sizeof(diff), oldsum); } -static inline bool ip_vs_checksum_needed(struct sk_buff *skb, int af) +static inline bool ip_vs_checksum_needed(struct sk_buff *skb) { /* Checksum unnecessary or already validated? */ if (skb_csum_unnecessary(skb)) return false; - /* LOCAL_OUT ? */ - if (!skb->dev || skb->dev->flags & IFF_LOOPBACK) + /* Locally generated ? */ + if (!skb->dev) return false; - /* !LOCAL_IN (FORWARD) ? */ - if (af == AF_INET6) { - if (!(dst_rt6_info(skb_dst(skb))->rt6i_flags & RTF_LOCAL)) - return false; - } else { - if (!(skb_rtable(skb)->rt_flags & RTCF_LOCAL)) - return false; - } return true; } static inline bool ip_vs_checksum_common_check(struct sk_buff *skb, int offset, int proto, int af) { - if (!ip_vs_checksum_needed(skb, af)) + if (!ip_vs_checksum_needed(skb)) return true; + /* Validate csum even for FORWARD */ return !nf_checksum(skb, NF_INET_LOCAL_IN, offset, proto, af); } diff --git a/net/netfilter/ipvs/ip_vs_proto_sctp.c b/net/netfilter/ipvs/ip_vs_proto_sctp.c index 3dbd3096e163..c80567c73469 100644 --- a/net/netfilter/ipvs/ip_vs_proto_sctp.c +++ b/net/netfilter/ipvs/ip_vs_proto_sctp.c @@ -193,7 +193,7 @@ sctp_csum_check(int af, struct sk_buff *skb, struct ip_vs_protocol *pp, struct sctphdr *sh; __le32 cmp, val; - if (!ip_vs_checksum_needed(skb, af)) + if (!ip_vs_checksum_needed(skb)) return 1; sh = (struct sctphdr *)(skb->data + sctphoff); cmp = sh->checksum; -- cgit