Skip to content

Commit 5f59819

Browse files
committed
eBPF correctness and coverage improvements
- Validate IPv4 IHL field (reject malformed headers with ihl<5, validate full header including options) and use a transport pointer derived from the variable-length IP header for TCP/UDP/ICMP dispatch - Add ip6_send_skb kprobe to track outbound IPv6 UDP traffic alongside the existing ip_send_skb IPv4 hook; guard both with msglen>0 check - Fix __icmp_send and icmp6_send direction: swap src/dst IPs so the entry reflects the ICMP error sender (us) rather than the triggering packet sender; also set type/code for icmp6_send (was missing) - Warn at startup when mutually-exclusive capture modes are combined so the implicit precedence (cgroup > kprobes > xdp > TC) is surfaced - Show empty PID cell instead of "0" in the TUI for kernel/unknown flows - Restrict fmt-bpf task glob to *.bpf.c to avoid touching header files
1 parent a669977 commit 5f59819

File tree

8 files changed

+125
-17
lines changed

8 files changed

+125
-17
lines changed

Taskfile.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ tasks:
4949

5050
fmt-bpf:
5151
cmds:
52-
- clang-format -i ./bpf/*
53-
- gsed -i 's,[[:space:]]*go:build,go:build,g' ./bpf/*
52+
- clang-format -i ./bpf/*.bpf.c
53+
- gsed -i 's,[[:space:]]*go:build,go:build,g' ./bpf/*.bpf.c
5454

5555
modernize:
5656
cmds:

bpf/counter.bpf.c

Lines changed: 91 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,27 @@ static const __u8 ip4in6[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0xff, 0xff};
8484
*/
8585
static inline __attribute__((always_inline)) int
8686
process_ip4(struct iphdr *ip4, void *data_end, statkey *key) {
87-
// validate IPv4 size
87+
// validate IPv4 fixed header size
8888
if ((void *)ip4 + sizeof(*ip4) > data_end) {
8989
return NOK;
9090
}
9191

92+
// ihl is the header length in 32-bit words (4-bit field, range 0–15).
93+
// Values < 5 are malformed; values > 5 indicate IPv4 options are present.
94+
__u8 ihl = ip4->ihl;
95+
if (ihl < 5) {
96+
return NOK;
97+
}
98+
__u32 ip4_hdr_len = (__u32)ihl * 4;
99+
100+
// validate the full IP header (including any options) fits in the packet
101+
if ((void *)ip4 + ip4_hdr_len > data_end) {
102+
return NOK;
103+
}
104+
105+
// transport header starts after the variable-length IP header
106+
void *transport = (void *)ip4 + ip4_hdr_len;
107+
92108
// convert to V4MAPPED address
93109
__builtin_memcpy(key->srcip.s6_addr, ip4in6, sizeof(ip4in6));
94110
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4->saddr,
@@ -103,7 +119,7 @@ process_ip4(struct iphdr *ip4, void *data_end, statkey *key) {
103119

104120
switch (ip4->protocol) {
105121
case IPPROTO_TCP: {
106-
struct tcphdr *tcp = (void *)ip4 + sizeof(*ip4);
122+
struct tcphdr *tcp = transport;
107123

108124
// validate TCP size
109125
if ((void *)tcp + sizeof(*tcp) > data_end) {
@@ -116,7 +132,7 @@ process_ip4(struct iphdr *ip4, void *data_end, statkey *key) {
116132
break;
117133
}
118134
case IPPROTO_UDP: {
119-
struct udphdr *udp = (void *)ip4 + sizeof(*ip4);
135+
struct udphdr *udp = transport;
120136

121137
// validate UDP size
122138
if ((void *)udp + sizeof(*udp) > data_end) {
@@ -129,7 +145,7 @@ process_ip4(struct iphdr *ip4, void *data_end, statkey *key) {
129145
break;
130146
}
131147
case IPPROTO_ICMP: {
132-
struct icmphdr *icmp = (void *)ip4 + sizeof(*ip4);
148+
struct icmphdr *icmp = transport;
133149

134150
// validate ICMP size
135151
if ((void *)icmp + sizeof(*icmp) > data_end) {
@@ -520,12 +536,14 @@ process_tcp(bool receive, struct sock *sk, statkey *key, pid_t pid) {
520536
// convert to V4MAPPED address
521537
__be32 ip4_src = BPF_CORE_READ(sk, __sk_common.skc_rcv_saddr);
522538
__builtin_memcpy(key->srcip.s6_addr, ip4in6, sizeof(ip4in6));
523-
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4_src, sizeof(ip4_src));
539+
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4_src,
540+
sizeof(ip4_src));
524541

525542
// convert to V4MAPPED address
526543
__be32 ip4_dst = BPF_CORE_READ(sk, __sk_common.skc_daddr);
527544
__builtin_memcpy(key->dstip.s6_addr, ip4in6, sizeof(ip4in6));
528-
__builtin_memcpy(key->dstip.s6_addr + sizeof(ip4in6), &ip4_dst, sizeof(ip4_dst));
545+
__builtin_memcpy(key->dstip.s6_addr + sizeof(ip4in6), &ip4_dst,
546+
sizeof(ip4_dst));
529547

530548
break;
531549
}
@@ -597,12 +615,14 @@ process_udp_recv(bool receive, struct sk_buff *skb, statkey *key, pid_t pid) {
597615
// convert to V4MAPPED address
598616
__be32 ip4_src = BPF_CORE_READ(iphdr, saddr);
599617
__builtin_memcpy(key->srcip.s6_addr, ip4in6, sizeof(ip4in6));
600-
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4_src, sizeof(ip4_src));
618+
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4_src,
619+
sizeof(ip4_src));
601620

602621
// convert to V4MAPPED address
603622
__be32 ip4_dst = BPF_CORE_READ(iphdr, daddr);
604623
__builtin_memcpy(key->dstip.s6_addr, ip4in6, sizeof(ip4in6));
605-
__builtin_memcpy(key->dstip.s6_addr + sizeof(ip4in6), &ip4_dst, sizeof(ip4_dst));
624+
__builtin_memcpy(key->dstip.s6_addr + sizeof(ip4in6), &ip4_dst,
625+
sizeof(ip4_dst));
606626
break;
607627
}
608628
case ETH_P_IPV6: {
@@ -664,12 +684,14 @@ process_icmp4(struct sk_buff *skb, statkey *key, pid_t pid) {
664684
// convert to V4MAPPED address
665685
__be32 ip4_src = BPF_CORE_READ(iphdr, saddr);
666686
__builtin_memcpy(key->srcip.s6_addr, ip4in6, sizeof(ip4in6));
667-
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4_src, sizeof(ip4_src));
687+
__builtin_memcpy(key->srcip.s6_addr + sizeof(ip4in6), &ip4_src,
688+
sizeof(ip4_src));
668689

669690
// convert to V4MAPPED address
670691
__be32 ip4_dst = BPF_CORE_READ(iphdr, daddr);
671692
__builtin_memcpy(key->dstip.s6_addr, ip4in6, sizeof(ip4in6));
672-
__builtin_memcpy(key->dstip.s6_addr + sizeof(ip4in6), &ip4_dst, sizeof(ip4_dst));
693+
__builtin_memcpy(key->dstip.s6_addr + sizeof(ip4in6), &ip4_dst,
694+
sizeof(ip4_dst));
673695

674696
// store ICMP type in src port
675697
key->src_port = BPF_CORE_READ(icmphdr, type);
@@ -985,7 +1007,47 @@ int BPF_KPROBE(ip_send_skb, struct net *net, struct sk_buff *skb) {
9851007
key.cgroupid = bpf_get_current_cgroup_id();
9861008

9871009
size_t msglen = process_udp_send(skb, &key, pid);
988-
update_val(&key, msglen);
1010+
if (msglen > 0)
1011+
update_val(&key, msglen);
1012+
1013+
return 0;
1014+
}
1015+
1016+
/**
1017+
* Hook function for kprobe on ip6_send_skb function.
1018+
*
1019+
* Populates the statkey structure with information from the socket and the
1020+
* process ID associated with the socket, and updates the packet and byte
1021+
* counters in the packet count map.
1022+
*
1023+
* @param skb pointer to the socket buffer
1024+
*
1025+
* @return 0
1026+
*
1027+
* @throws none
1028+
*/
1029+
SEC("kprobe/ip6_send_skb")
1030+
int BPF_KPROBE(ip6_send_skb, struct sk_buff *skb) {
1031+
struct ipv6hdr *ip6hdr =
1032+
(struct ipv6hdr *)(BPF_CORE_READ(skb, head) +
1033+
BPF_CORE_READ(skb, network_header));
1034+
if (BPF_CORE_READ(ip6hdr, nexthdr) != IPPROTO_UDP) {
1035+
return 0;
1036+
}
1037+
1038+
statkey key;
1039+
__builtin_memset(&key, 0, sizeof(key));
1040+
1041+
pid_t pid = bpf_get_current_pid_tgid() & 0xFFFFFFFF;
1042+
if (pid > 0) {
1043+
bpf_get_current_comm(&key.comm, sizeof(key.comm));
1044+
}
1045+
1046+
key.cgroupid = bpf_get_current_cgroup_id();
1047+
1048+
size_t msglen = process_udp_send(skb, &key, pid);
1049+
if (msglen > 0)
1050+
update_val(&key, msglen);
9891051

9901052
return 0;
9911053
}
@@ -1059,9 +1121,14 @@ int BPF_KPROBE(__icmp_send, struct sk_buff *skb, __u8 type, __u8 code,
10591121
key.cgroupid = bpf_get_current_cgroup_id();
10601122

10611123
size_t msglen = process_icmp4(skb, &key, pid);
1062-
/* __icmp_send is called with the triggering packet's skb; its
1063-
* transport_header points to that packet's transport layer (e.g. TCP/UDP),
1064-
* not to an ICMP header. Use the kprobe arguments directly for type/code. */
1124+
/* __icmp_send is called with the triggering packet's skb, so src/dst IPs
1125+
* come from the original packet (src=sender, dst=us). Swap them to reflect
1126+
* the actual ICMP error direction (src=us, dst=original sender).
1127+
* The transport_header also points to the triggering transport layer, not
1128+
* an ICMP header, so use the kprobe arguments directly for type/code. */
1129+
struct in6_addr tmp_ip = key.srcip;
1130+
key.srcip = key.dstip;
1131+
key.dstip = tmp_ip;
10651132
key.src_port = type;
10661133
key.dst_port = code;
10671134
update_val(&key, msglen);
@@ -1099,6 +1166,16 @@ int BPF_KPROBE(icmp6_send, struct sk_buff *skb, __u8 type, __u8 code,
10991166
key.cgroupid = bpf_get_current_cgroup_id();
11001167

11011168
size_t msglen = process_icmp6(skb, &key, pid);
1169+
/* icmp6_send is called with the triggering packet's skb, so src/dst IPs
1170+
* come from the original packet (src=sender, dst=us). Swap them to reflect
1171+
* the actual ICMPv6 error direction (src=us, dst=original sender).
1172+
* The transport_header also points to the triggering transport layer, not
1173+
* an ICMPv6 header, so use the kprobe arguments directly for type/code. */
1174+
struct in6_addr tmp_ip = key.srcip;
1175+
key.srcip = key.dstip;
1176+
key.dstip = tmp_ip;
1177+
key.src_port = type;
1178+
key.dst_port = code;
11021179
update_val(&key, msglen);
11031180

11041181
return 0;

counter_arm64_bpfel.go

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

counter_arm64_bpfel.o

9.94 KB
Binary file not shown.

counter_x86_bpfel.go

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

counter_x86_bpfel.o

9.93 KB
Binary file not shown.

main.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,25 @@ func main() {
8989
}
9090
}()
9191

92+
// Warn when multiple mutually-exclusive capture modes are combined.
93+
// The switch below silently picks the first matching case, so the user
94+
// might not realise one of their flags is being ignored.
95+
{
96+
captureModes := 0
97+
if *useCGroup != "" {
98+
captureModes++
99+
}
100+
if *useKProbes {
101+
captureModes++
102+
}
103+
if *useXDP {
104+
captureModes++
105+
}
106+
if captureModes > 1 {
107+
log.Printf("Warning: multiple capture modes specified; precedence is --cgroup > --kprobes > --xdp > TC (default)")
108+
}
109+
}
110+
92111
switch {
93112
case *useCGroup != "":
94113
cGroupCacheInit()
@@ -110,6 +129,7 @@ func main() {
110129
{kprobe: "tcp_sendmsg", prog: objsCounter.TcpSendmsg},
111130
{kprobe: "tcp_cleanup_rbuf", prog: objsCounter.TcpCleanupRbuf},
112131
{kprobe: "ip_send_skb", prog: objsCounter.IpSendSkb},
132+
{kprobe: "ip6_send_skb", prog: objsCounter.Ip6SendSkb},
113133
{kprobe: "skb_consume_udp", prog: objsCounter.SkbConsumeUdp},
114134
{kprobe: "__icmp_send", prog: objsCounter.IcmpSend},
115135
{kprobe: "icmp6_send", prog: objsCounter.Icmp6Send},

tui.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,12 @@ func updateStatsTable(app *tview.Application, table *tview.Table, tableSortIdx *
274274

275275
// populate pid, comm and cgroup
276276
if *useKProbes || *useCGroup != "" {
277-
table.SetCell(i+1, 8, tview.NewTableCell(strconv.FormatInt(int64(v.Pid), 10)).
277+
pidStr := ""
278+
if v.Pid > 0 {
279+
pidStr = strconv.FormatInt(int64(v.Pid), 10)
280+
}
281+
282+
table.SetCell(i+1, 8, tview.NewTableCell(pidStr).
278283
SetTextColor(tcell.ColorWhite).
279284
SetExpansion(1))
280285

0 commit comments

Comments
 (0)