From daeb6a8f3b00d3b6f40593d80ab6be4f6d3d968d Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 26 Feb 2025 18:34:36 +0000 Subject: [PATCH 1/2] ipv4: icmp: do not process ICMP_EXT_ECHOREPLY for broadcast/multicast addresses There is no point processing ICMP_EXT_ECHOREPLY for routes which would drop ICMP_ECHOREPLY (RFC 1122 3.2.2.6, 3.2.2.8) This seems an oversight of the initial implementation. Signed-off-by: Eric Dumazet Reviewed-by: David Ahern Link: https://patch.msgid.link/20250226183437.1457318-2-edumazet@google.com Signed-off-by: Jakub Kicinski --- net/ipv4/icmp.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 799775ba97d4..058d4c1e300d 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -1248,22 +1248,6 @@ int icmp_rcv(struct sk_buff *skb) goto reason_check; } - if (icmph->type == ICMP_EXT_ECHOREPLY) { - reason = ping_rcv(skb); - goto reason_check; - } - - /* - * 18 is the highest 'known' ICMP type. Anything else is a mystery - * - * RFC 1122: 3.2.2 Unknown ICMP messages types MUST be silently - * discarded. - */ - if (icmph->type > NR_ICMP_TYPES) { - reason = SKB_DROP_REASON_UNHANDLED_PROTO; - goto error; - } - /* * Parse the ICMP message */ @@ -1290,6 +1274,22 @@ int icmp_rcv(struct sk_buff *skb) } } + if (icmph->type == ICMP_EXT_ECHOREPLY) { + reason = ping_rcv(skb); + goto reason_check; + } + + /* + * 18 is the highest 'known' ICMP type. Anything else is a mystery + * + * RFC 1122: 3.2.2 Unknown ICMP messages types MUST be silently + * discarded. + */ + if (icmph->type > NR_ICMP_TYPES) { + reason = SKB_DROP_REASON_UNHANDLED_PROTO; + goto error; + } + reason = icmp_pointers[icmph->type].handler(skb); reason_check: if (!reason) { From a7e38208fe71498102de3ea9d20cfe847cdb6893 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Wed, 26 Feb 2025 18:34:37 +0000 Subject: [PATCH 2/2] inet: ping: avoid skb_clone() dance in ping_rcv() ping_rcv() callers currently call skb_free() or consume_skb(), forcing ping_rcv() to clone the skb. After this patch ping_rcv() is now 'consuming' the original skb, either moving to a socket receive queue, or dropping it. Signed-off-by: Eric Dumazet Reviewed-by: David Ahern Link: https://patch.msgid.link/20250226183437.1457318-3-edumazet@google.com Signed-off-by: Jakub Kicinski --- net/ipv4/icmp.c | 5 +++-- net/ipv4/ping.c | 20 +++++--------------- net/ipv6/icmp.c | 7 ++----- 3 files changed, 10 insertions(+), 22 deletions(-) diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c index 058d4c1e300d..717cb7d3607a 100644 --- a/net/ipv4/icmp.c +++ b/net/ipv4/icmp.c @@ -1274,9 +1274,10 @@ int icmp_rcv(struct sk_buff *skb) } } - if (icmph->type == ICMP_EXT_ECHOREPLY) { + if (icmph->type == ICMP_EXT_ECHOREPLY || + icmph->type == ICMP_ECHOREPLY) { reason = ping_rcv(skb); - goto reason_check; + return reason ? NET_RX_DROP : NET_RX_SUCCESS; } /* diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c index 85d09f2ecadc..c14baa6589c7 100644 --- a/net/ipv4/ping.c +++ b/net/ipv4/ping.c @@ -966,10 +966,9 @@ EXPORT_SYMBOL_GPL(ping_queue_rcv_skb); enum skb_drop_reason ping_rcv(struct sk_buff *skb) { - enum skb_drop_reason reason = SKB_DROP_REASON_NO_SOCKET; - struct sock *sk; struct net *net = dev_net(skb->dev); struct icmphdr *icmph = icmp_hdr(skb); + struct sock *sk; /* We assume the packet has already been checked by icmp_rcv */ @@ -980,20 +979,11 @@ enum skb_drop_reason ping_rcv(struct sk_buff *skb) skb_push(skb, skb->data - (u8 *)icmph); sk = ping_lookup(net, skb, ntohs(icmph->un.echo.id)); - if (sk) { - struct sk_buff *skb2 = skb_clone(skb, GFP_ATOMIC); - - pr_debug("rcv on socket %p\n", sk); - if (skb2) - reason = __ping_queue_rcv_skb(sk, skb2); - else - reason = SKB_DROP_REASON_NOMEM; - } - - if (reason) - pr_debug("no socket, dropping\n"); + if (sk) + return __ping_queue_rcv_skb(sk, skb); - return reason; + kfree_skb_reason(skb, SKB_DROP_REASON_NO_SOCKET); + return SKB_DROP_REASON_NO_SOCKET; } EXPORT_SYMBOL_GPL(ping_rcv); diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c index 4d14ab7f7e99..3fd19a84b358 100644 --- a/net/ipv6/icmp.c +++ b/net/ipv6/icmp.c @@ -957,12 +957,9 @@ static int icmpv6_rcv(struct sk_buff *skb) break; case ICMPV6_ECHO_REPLY: - reason = ping_rcv(skb); - break; - case ICMPV6_EXT_ECHO_REPLY: - reason = ping_rcv(skb); - break; + ping_rcv(skb); + return 0; case ICMPV6_PKT_TOOBIG: /* BUGGG_FUTURE: if packet contains rthdr, we cannot update