* [PATCH net-next 1/2] ipv4: icmp: do not process ICMP_EXT_ECHOREPLY for broadcast/multicast addresses
2025-02-26 18:34 [PATCH net-next 0/2] inet: ping: remove extra skb_clone()/consume_skb() Eric Dumazet
@ 2025-02-26 18:34 ` Eric Dumazet
2025-02-28 16:28 ` David Ahern
2025-02-26 18:34 ` [PATCH net-next 2/2] inet: ping: avoid skb_clone() dance in ping_rcv() Eric Dumazet
2025-02-28 22:50 ` [PATCH net-next 0/2] inet: ping: remove extra skb_clone()/consume_skb() patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2025-02-26 18:34 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, netdev, eric.dumazet, David Ahern, Eric Dumazet
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 <edumazet@google.com>
---
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 799775ba97d4f1bad1f070fb2a2596650df177af..058d4c1e300d0c0be7a04fd67e8e39924dfcd2cc 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) {
--
2.48.1.658.g4767266eb4-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH net-next 2/2] inet: ping: avoid skb_clone() dance in ping_rcv()
2025-02-26 18:34 [PATCH net-next 0/2] inet: ping: remove extra skb_clone()/consume_skb() Eric Dumazet
2025-02-26 18:34 ` [PATCH net-next 1/2] ipv4: icmp: do not process ICMP_EXT_ECHOREPLY for broadcast/multicast addresses Eric Dumazet
@ 2025-02-26 18:34 ` Eric Dumazet
2025-02-28 16:30 ` David Ahern
2025-02-28 22:50 ` [PATCH net-next 0/2] inet: ping: remove extra skb_clone()/consume_skb() patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Eric Dumazet @ 2025-02-26 18:34 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Paolo Abeni
Cc: Simon Horman, netdev, eric.dumazet, David Ahern, Eric Dumazet
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 <edumazet@google.com>
---
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 058d4c1e300d0c0be7a04fd67e8e39924dfcd2cc..717cb7d3607a1c77a3f54b56d2bb98b1064dd878 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 85d09f2ecadcb690f01985771afa37ce2cd0befc..c14baa6589c748026b49416688cbea399e6d461a 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 4d14ab7f7e99f152cd5f5adaa023f0280957f275..3fd19a84b358d169bbdc351c43ede830c60afcf3 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
--
2.48.1.658.g4767266eb4-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread