quoted

Introduce a tracepoint for icmp_send, which can help users to get more
detail information conveniently when icmp abnormal events happen.

1. Giving an usecase example:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

=3D=3D=3D=3D=3D

quoted

When an application experiences packet loss due to an unreachable UDP
destination port, the kernel will send an exception message through the
icmp_send function. By adding a trace point for icmp_send, developers or
system administrators can obtain detailed information about the UDP
packet loss, including the type, code, source address, destination addres=

s,

quoted

source port, and destination port. This facilitates the trouble-shooting
of UDP packet loss issues especially for those network-service
applications.

2. Operation Instructions:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=

=3D=3D

quoted

Switch to the tracing directory.
        cd /sys/kernel/tracing
Filter for destination port unreachable.
        echo "type=3D=3D3 && code=3D=3D3" > events/icmp/icmp_send/filter
Enable trace event.
        echo 1 > events/icmp/icmp_send/enable

3. Result View:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
 udp_client_erro-11370   [002] ...s.12   124.728002:
 icmp_send: icmp_send: type=3D3, code=3D3.
 From 127.0.0.1:41895 to 127.0.0.1:6666 ulen=3D23
 skbaddr=3D00000000589b167a

Changelog
---------
v2->v3:
Some fixes according to
https://lore.kernel.org/all/20240319102549.7f7f6f53@gandalf.local.home/ (local)
1. Change the tracking directory to/sys/kernel/tracking.
2. Adjust the layout of the TP-STRUCT_entry parameter structure.

v1->v2:
Some fixes according to
https://lore.kernel.org/all/CANn89iL-y9e_VFpdw=3DsZtRnKRu_tnUwqHuFQTJvJsv=

-nz1xPDw@mail.gmail.com/

quoted

1. adjust the trace_icmp_send() to more protocols than UDP.
2. move the calling of trace_icmp_send after sanity checks
in __icmp_send().

Signed-off-by: Peilin He<redacted>
Reviewed-by: xu xin <xu.xin16@zte.com.cn>
Reviewed-by: Yunkai Zhang <redacted>
Cc: Yang Yang <yang.yang29@zte.com.cn>
Cc: Liu Chun <redacted>
Cc: Xuexin Jiang <redacted>
---
 include/trace/events/icmp.h | 64 +++++++++++++++++++++++++++++++++++++
 net/ipv4/icmp.c             |  4 +++
 2 files changed, 68 insertions(+)
 create mode 100644 include/trace/events/icmp.h

diff --git a/include/trace/events/icmp.h b/include/trace/events/icmp.h
new file mode 100644
index 000000000000..2098d4b1b12e
--- /dev/null
+++ b/include/trace/events/icmp.h

@@ -0,0 +1,64 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM icmp
+
+#if !defined(_TRACE_ICMP_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_ICMP_H
+
+#include <linux/icmp.h>
+#include <linux/tracepoint.h>
+
+TRACE_EVENT(icmp_send,
+
+               TP_PROTO(const struct sk_buff *skb, int type, int code),
+
+               TP_ARGS(skb, type, code),
+
+               TP_STRUCT__entry(
+                       __field(const void *, skbaddr)
+                       __field(int, type)
+                       __field(int, code)
+                       __array(__u8, saddr, 4)
+                       __array(__u8, daddr, 4)
+                       __field(__u16, sport)
+                       __field(__u16, dport)
+                       __field(unsigned short, ulen)
+               ),
+
+               TP_fast_assign(
+                       struct iphdr *iph =3D ip_hdr(skb);
+                       int proto_4 =3D iph->protocol;
+                       __be32 *p32;
+
+                       __entry->skbaddr =3D skb;
+                       __entry->type =3D type;
+                       __entry->code =3D code;
+
+                       if (proto_4 =3D=3D IPPROTO_UDP) {
+                               struct udphdr *uh =3D udp_hdr(skb);
+                               __entry->sport =3D ntohs(uh->source);
+                               __entry->dport =3D ntohs(uh->dest);
+                               __entry->ulen =3D ntohs(uh->len);

This is completely bogus.

Adding tracepoints is ok if there are no side effects like bugs :/

At this point there is no guarantee the UDP header is complete/present
in skb->head

Look at the existing checks between lines 619 and 623

Then audit all icmp_send() callers, and ask yourself if UDP packets
can not be malicious (like with a truncated UDP header)

Yeah, you are correct. Directly parsing udphdr through the sdk may
conceal bugs, such as illegal skb. To handle such exceptional scenarios,
we can determine the legitimacy of skb by checking whether the position
of the uh pointer is out of bounds.

Perhaps it could be modified like this:	
	struct udphdr *uh = udp_hdr(skb);
	
	if (proto_4 != IPPROTO_UDP || (u8 *)uh < skb->head ||
		(u8 *)uh + sizeof(struct udphdr) > skb_tail_pointer(skb)) {
		__entry->sport = 0;
		__entry->dport = 0;
		__entry->ulen = 0;
	} else {
		__entry->sport = ntohs(uh->source);
		__entry->dport = ntohs(uh->dest);
		__entry->ulen = ntohs(uh->len);
	}
	
Additionally, the validity of all parameters in the current patch has been
confirmed without any issues. Thank you once again for your reminder.