Skip to content

Commit 483f80e

Browse files
committed
Fix ICMP/ICMPv6 send hooks: inline IP extraction and improve size accounting
In __icmp_send and icmp6_send, process_icmp4()/process_icmp6() were called to populate IPs, but the skb transport_header in those hooks points to the triggering TCP/UDP layer, not an ICMP header. Replace those calls with direct inline IP extraction from the network header, eliminating the garbage type/code reads that were immediately overwritten. Also fix two size accounting bugs: - __icmp_send: validate ihl >= 5 before computing ihl_bytes; a zero return from BPF_CORE_READ_BITFIELD_PROBED was silently producing a wrong msglen - icmp6_send: replace the hardcoded 56-byte constant with a proper RFC 4443 section 2.4 computation capping the included original packet at 1232 bytes (1280 min MTU - 40 outer IPv6 - 8 ICMPv6 header) Disable raw_sendmsg and rawv6_sendmsg kprobes (#if 0 / commented out) as they are not ready for production use.
1 parent 7b6ba94 commit 483f80e

6 files changed

Lines changed: 56 additions & 35 deletions

File tree

bpf/kprobe.bpf.c

Lines changed: 54 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -248,15 +248,30 @@ int BPF_KPROBE(__icmp_send, struct sk_buff *skb, __u8 type, __u8 code,
248248

249249
key.cgroupid = get_current_cgroup_id();
250250

251-
/* process_icmp4 populates src/dst IPs from the triggering packet's IP
252-
* header; its returned msglen (triggering payload) is not used here. */
253-
process_icmp4(skb, &key, pid);
251+
/* Extract src/dst IPs directly from the triggering packet's IP header.
252+
* process_icmp4() cannot be used here: the skb's transport_header points to
253+
* the triggering transport layer (TCP/UDP), not an ICMP header, so calling
254+
* it would read garbage type/code that we would then have to overwrite. */
255+
struct iphdr *iphdr = (struct iphdr *)(BPF_CORE_READ(skb, head) +
256+
BPF_CORE_READ(skb, network_header));
257+
258+
__be32 ip4_src = BPF_CORE_READ(iphdr, saddr);
259+
__builtin_memcpy(key.srcip.s6_addr, ip4in6, sizeof(ip4in6));
260+
__builtin_memcpy(key.srcip.s6_addr + sizeof(ip4in6), &ip4_src,
261+
sizeof(ip4_src));
262+
263+
__be32 ip4_dst = BPF_CORE_READ(iphdr, daddr);
264+
__builtin_memcpy(key.dstip.s6_addr, ip4in6, sizeof(ip4in6));
265+
__builtin_memcpy(key.dstip.s6_addr + sizeof(ip4in6), &ip4_dst,
266+
sizeof(ip4_dst));
267+
268+
key.proto = IPPROTO_ICMP;
269+
key.pid = pid;
254270

255271
/* __icmp_send is called with the triggering packet's skb, so src/dst IPs
256272
* come from the original packet (src=sender, dst=us). Swap them to reflect
257273
* the actual ICMP error direction (src=us, dst=original sender).
258-
* The transport_header also points to the triggering transport layer, not
259-
* an ICMP header, so use the kprobe arguments directly for type/code. */
274+
* Use kprobe arguments directly for type/code. */
260275
struct in6_addr tmp_ip = key.srcip;
261276
key.srcip = key.dstip;
262277
key.dstip = tmp_ip;
@@ -265,13 +280,13 @@ int BPF_KPROBE(__icmp_send, struct sk_buff *skb, __u8 type, __u8 code,
265280

266281
/* Compute the actual ICMP error response size per RFC 792 / RFC 1812:
267282
* 8-byte ICMP header + original IP header + first 8 bytes of original data.
268-
* Using the triggering packet's tot_len (as process_icmp4 did) over-counts
269-
* by including the full original payload instead of just the 8-byte excerpt.
270-
*/
271-
struct iphdr *iphdr = (struct iphdr *)(BPF_CORE_READ(skb, head) +
272-
BPF_CORE_READ(skb, network_header));
283+
* Validate ihl >= 5 (minimum 20-byte header) before using it. */
273284
__u16 tot_len = bpf_ntohs(BPF_CORE_READ(iphdr, tot_len));
274-
__u16 ihl_bytes = (__u16)(BPF_CORE_READ_BITFIELD_PROBED(iphdr, ihl) * 4);
285+
__u16 ihl_raw = (__u16)BPF_CORE_READ_BITFIELD_PROBED(iphdr, ihl);
286+
if (ihl_raw < 5) {
287+
return 0;
288+
}
289+
__u16 ihl_bytes = ihl_raw * 4;
275290
__u16 payload = (ihl_bytes <= tot_len) ? (tot_len - ihl_bytes) : 0;
276291
size_t msglen =
277292
sizeof(struct icmphdr) + ihl_bytes + (payload > 8 ? 8 : payload);
@@ -310,26 +325,40 @@ int BPF_KPROBE(icmp6_send, struct sk_buff *skb, __u8 type, __u8 code,
310325

311326
key.cgroupid = get_current_cgroup_id();
312327

313-
/* process_icmp6 populates src/dst IPs from the triggering packet's IPv6
314-
* header; its returned msglen (triggering payload_len) is not used here. */
315-
process_icmp6(skb, &key, pid);
328+
/* Extract src/dst IPs directly from the triggering packet's IPv6 header.
329+
* process_icmp6() cannot be used here: the skb's transport_header points to
330+
* the triggering transport layer (TCP/UDP), not an ICMPv6 header, so calling
331+
* it would read garbage type/code that we would then have to overwrite. */
332+
struct ipv6hdr *iphdr =
333+
(struct ipv6hdr *)(BPF_CORE_READ(skb, head) +
334+
BPF_CORE_READ(skb, network_header));
335+
336+
BPF_CORE_READ_INTO(&key.srcip, iphdr, saddr);
337+
BPF_CORE_READ_INTO(&key.dstip, iphdr, daddr);
338+
339+
key.proto = IPPROTO_ICMPV6;
340+
key.pid = pid;
316341

317342
/* icmp6_send is called with the triggering packet's skb, so src/dst IPs
318343
* come from the original packet (src=sender, dst=us). Swap them to reflect
319344
* the actual ICMPv6 error direction (src=us, dst=original sender).
320-
* The transport_header also points to the triggering transport layer, not
321-
* an ICMPv6 header, so use the kprobe arguments directly for type/code. */
345+
* Use kprobe arguments directly for type/code. */
322346
struct in6_addr tmp_ip = key.srcip;
323347
key.srcip = key.dstip;
324348
key.dstip = tmp_ip;
325349
key.src_port = type;
326350
key.dst_port = code;
327351

328-
/* Compute the actual ICMPv6 error response size per RFC 4443:
329-
* 8-byte ICMPv6 header + fixed 40-byte IPv6 header + first 8 bytes of
330-
* original data. The triggering payload_len (as process_icmp6 returned)
331-
* over-counts by including the full original payload. */
332-
size_t msglen = sizeof(struct icmp6hdr) + sizeof(struct ipv6hdr) + 8;
352+
/* Compute the ICMPv6 error response size per RFC 4443 section 2.4:
353+
* 8-byte ICMPv6 header + as much of the original IPv6 packet as fits within
354+
* the minimum IPv6 MTU (1280 bytes). Max includable body:
355+
* 1280 - 40 (outer IPv6 header) - 8 (ICMPv6 header) = 1232 bytes. */
356+
__u16 payload_len = bpf_ntohs(BPF_CORE_READ(iphdr, payload_len));
357+
__u32 orig_len = (__u32)sizeof(struct ipv6hdr) + payload_len;
358+
__u32 max_body =
359+
1280U - (__u32)sizeof(struct ipv6hdr) - (__u32)sizeof(struct icmp6hdr);
360+
__u32 body = orig_len < max_body ? orig_len : max_body;
361+
size_t msglen = sizeof(struct icmp6hdr) + body;
333362

334363
update_val(&key, msglen);
335364

@@ -400,6 +429,7 @@ int BPF_KPROBE(icmpv6_rcv, struct sk_buff *skb) {
400429
return 0;
401430
}
402431

432+
#if 0
403433
/**
404434
* Hook function for kprobe on raw_sendmsg function.
405435
*
@@ -428,7 +458,9 @@ int BPF_KPROBE(raw_sendmsg, struct sock *sk, struct msghdr *msg, size_t len) {
428458

429459
return 0;
430460
}
461+
#endif
431462

463+
#if 0
432464
/**
433465
* Hook function for kprobe on rawv6_sendmsg function.
434466
*
@@ -457,5 +489,6 @@ int BPF_KPROBE(rawv6_sendmsg, struct sock *sk, struct msghdr *msg, size_t len) {
457489

458490
return 0;
459491
}
492+
#endif
460493

461494
char __license[] SEC("license") = "Dual MIT/GPL";

kprobe_arm64_bpfel.go

Lines changed: 0 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

kprobe_arm64_bpfel.o

-16.4 KB
Binary file not shown.

kprobe_x86_bpfel.go

Lines changed: 0 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

kprobe_x86_bpfel.o

-16.4 KB
Binary file not shown.

main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -169,8 +169,8 @@ func main() {
169169
{kprobe: "icmp6_send", prog: objsKprobe.Icmp6Send},
170170
{kprobe: "icmp_rcv", prog: objsKprobe.IcmpRcv},
171171
{kprobe: "icmpv6_rcv", prog: objsKprobe.Icmpv6Rcv},
172-
{kprobe: "raw_sendmsg", prog: objsKprobe.RawSendmsg},
173-
{kprobe: "rawv6_sendmsg", prog: objsKprobe.Rawv6Sendmsg},
172+
//{kprobe: "raw_sendmsg", prog: objsKprobe.RawSendmsg},
173+
//{kprobe: "rawv6_sendmsg", prog: objsKprobe.Rawv6Sendmsg},
174174
}
175175

176176
cGroupCacheInit()

0 commit comments

Comments
 (0)