Add coverage to the verifier tests and tests for reading bpf_sock fields to ensure that 32-bit, 16-bit, and 8-bit loads from dst_port field are allowed only at intended offsets and produce expected values. While 16-bit and 8-bit access to dst_port field is straight-forward, 32-bit wide loads need be allowed and produce a zero-padded 16-bit value for backward compatibility.
Unified · Split
| @@ -5574,7 +5574,8 @@ struct bpf_sock { · 5573 unmodified lines | ||
| 5574 | 5574 | __u32 src_ip4; |
| 5575 | 5575 | __u32 src_ip6[4]; |
| 5576 | 5576 | __u32 src_port; /* host byte order */ |
| 5577 | − __u32 dst_port; /* network byte order */ | |
| 5577 | + __be16 dst_port; /* network byte order */ | |
| 5578 | + __u16 :16; /* zero padding */ | |
| 5578 | 5579 | __u32 dst_ip4; |
| 5579 | 5580 | __u32 dst_ip6[4]; |
| 5580 | 5581 | __u32 state; |
| @@ -1,9 +1,11 @@ | ||
| 1 | 1 | // SPDX-License-Identifier: GPL-2.0 |
| 2 | 2 | /* Copyright (c) 2019 Facebook */ |
| 3 | 3 | |
| 4 | +#define _GNU_SOURCE | |
| 4 | 5 | #include <netinet/in.h> |
| 5 | 6 | #include <arpa/inet.h> |
| 6 | 7 | #include <unistd.h> |
| 8 | +#include <sched.h> | |
| 7 | 9 | #include <stdlib.h> |
| 8 | 10 | #include <string.h> |
| 9 | 11 | #include <errno.h> |
| @@ -20,6 +22,7 @@ · 10 unmodified lines | ||
| 20 | 22 | enum bpf_linum_array_idx { |
| 21 | 23 | EGRESS_LINUM_IDX, |
| 22 | 24 | INGRESS_LINUM_IDX, |
| 25 | + READ_SK_DST_PORT_LINUM_IDX, | |
| 23 | 26 | __NR_BPF_LINUM_ARRAY_IDX, |
| 24 | 27 | }; |
| 25 | 28 | |
| @@ -42,8 +45,16 @@ static __u64 child_cg_id; · 16 unmodified lines | ||
| 42 | 45 | static int linum_map_fd; |
| 43 | 46 | static __u32 duration; |
| 44 | 47 | |
| 45 | −static __u32 egress_linum_idx = EGRESS_LINUM_IDX; | |
| 46 | −static __u32 ingress_linum_idx = INGRESS_LINUM_IDX; | |
| 48 | +static bool create_netns(void) | |
| 49 | +{ | |
| 50 | + if (!ASSERT_OK(unshare(CLONE_NEWNET), "create netns")) | |
| 51 | + return false; | |
| 52 | + | |
| 53 | + if (!ASSERT_OK(system("ip link set dev lo up"), "bring up lo")) | |
| 54 | + return false; | |
| 55 | + | |
| 56 | + return true; | |
| 57 | +} | |
| 47 | 58 | |
| 48 | 59 | static void print_sk(const struct bpf_sock *sk, const char *prefix) |
| 49 | 60 | { |
| @@ -91,19 +102,24 @@ static void check_result(void) · 41 unmodified lines | ||
| 91 | 102 | { |
| 92 | 103 | struct bpf_tcp_sock srv_tp, cli_tp, listen_tp; |
| 93 | 104 | struct bpf_sock srv_sk, cli_sk, listen_sk; |
| 94 | − __u32 ingress_linum, egress_linum; | |
| 105 | + __u32 idx, ingress_linum, egress_linum, linum; | |
| 95 | 106 | int err; |
| 96 | 107 | |
| 97 | − err = bpf_map_lookup_elem(linum_map_fd, &egress_linum_idx, | |
| 98 | − &egress_linum); | |
| 108 | + idx = EGRESS_LINUM_IDX; | |
| 109 | + err = bpf_map_lookup_elem(linum_map_fd, &idx, &egress_linum); | |
| 99 | 110 | CHECK(err < 0, "bpf_map_lookup_elem(linum_map_fd)", |
| 100 | 111 | "err:%d errno:%d\n", err, errno); |
| 101 | 112 | |
| 102 | − err = bpf_map_lookup_elem(linum_map_fd, &ingress_linum_idx, | |
| 103 | − &ingress_linum); | |
| 113 | + idx = INGRESS_LINUM_IDX; | |
| 114 | + err = bpf_map_lookup_elem(linum_map_fd, &idx, &ingress_linum); | |
| 104 | 115 | CHECK(err < 0, "bpf_map_lookup_elem(linum_map_fd)", |
| 105 | 116 | "err:%d errno:%d\n", err, errno); |
| 106 | 117 | |
| 118 | + idx = READ_SK_DST_PORT_LINUM_IDX; | |
| 119 | + err = bpf_map_lookup_elem(linum_map_fd, &idx, &linum); | |
| 120 | + ASSERT_OK(err, "bpf_map_lookup_elem(linum_map_fd, READ_SK_DST_PORT_IDX)"); | |
| 121 | + ASSERT_EQ(linum, 0, "failure in read_sk_dst_port on line"); | |
| 122 | + | |
| 107 | 123 | memcpy(&srv_sk, &skel->bss->srv_sk, sizeof(srv_sk)); |
| 108 | 124 | memcpy(&srv_tp, &skel->bss->srv_tp, sizeof(srv_tp)); |
| 109 | 125 | memcpy(&cli_sk, &skel->bss->cli_sk, sizeof(cli_sk)); |
| @@ -262,7 +278,7 @@ static void test(void) · 152 unmodified lines | ||
| 262 | 278 | char buf[DATA_LEN]; |
| 263 | 279 | |
| 264 | 280 | /* Prepare listen_fd */ |
| 265 | − listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0, 0); | |
| 281 | + listen_fd = start_server(AF_INET6, SOCK_STREAM, "::1", 0xcafe, 0); | |
| 266 | 282 | /* start_server() has logged the error details */ |
| 267 | 283 | if (CHECK_FAIL(listen_fd == -1)) |
| 268 | 284 | goto done; |
| @@ -330,8 +346,12 @@ done: · 61 unmodified lines | ||
| 330 | 346 | |
| 331 | 347 | void serial_test_sock_fields(void) |
| 332 | 348 | { |
| 333 | − struct bpf_link *egress_link = NULL, *ingress_link = NULL; | |
| 334 | 349 | int parent_cg_fd = -1, child_cg_fd = -1; |
| 350 | + struct bpf_link *link; | |
| 351 | + | |
| 352 | + /* Use a dedicated netns to have a fixed listen port */ | |
| 353 | + if (!create_netns()) | |
| 354 | + return; | |
| 335 | 355 | |
| 336 | 356 | /* Create a cgroup, get fd, and join it */ |
| 337 | 357 | parent_cg_fd = test__join_cgroup(PARENT_CGROUP); |
| @@ -352,15 +372,20 @@ void serial_test_sock_fields(void) · 14 unmodified lines | ||
| 352 | 372 | if (CHECK(!skel, "test_sock_fields__open_and_load", "failed\n")) |
| 353 | 373 | goto done; |
| 354 | 374 | |
| 355 | − egress_link = bpf_program__attach_cgroup(skel->progs.egress_read_sock_fields, | |
| 356 | − child_cg_fd); | |
| 357 | − if (!ASSERT_OK_PTR(egress_link, "attach_cgroup(egress)")) | |
| 375 | + link = bpf_program__attach_cgroup(skel->progs.egress_read_sock_fields, child_cg_fd); | |
| 376 | + if (!ASSERT_OK_PTR(link, "attach_cgroup(egress_read_sock_fields)")) | |
| 377 | + goto done; | |
| 378 | + skel->links.egress_read_sock_fields = link; | |
| 379 | + | |
| 380 | + link = bpf_program__attach_cgroup(skel->progs.ingress_read_sock_fields, child_cg_fd); | |
| 381 | + if (!ASSERT_OK_PTR(link, "attach_cgroup(ingress_read_sock_fields)")) | |
| 358 | 382 | goto done; |
| 383 | + skel->links.ingress_read_sock_fields = link; | |
| 359 | 384 | |
| 360 | − ingress_link = bpf_program__attach_cgroup(skel->progs.ingress_read_sock_fields, | |
| 361 | − child_cg_fd); | |
| 362 | − if (!ASSERT_OK_PTR(ingress_link, "attach_cgroup(ingress)")) | |
| 385 | + link = bpf_program__attach_cgroup(skel->progs.read_sk_dst_port, child_cg_fd); | |
| 386 | + if (!ASSERT_OK_PTR(link, "attach_cgroup(read_sk_dst_port")) | |
| 363 | 387 | goto done; |
| 388 | + skel->links.read_sk_dst_port = link; | |
| 364 | 389 | |
| 365 | 390 | linum_map_fd = bpf_map__fd(skel->maps.linum_map); |
| 366 | 391 | sk_pkt_out_cnt_fd = bpf_map__fd(skel->maps.sk_pkt_out_cnt); |
| @@ -369,8 +394,7 @@ void serial_test_sock_fields(void) · 2 unmodified lines | ||
| 369 | 394 | test(); |
| 370 | 395 | |
| 371 | 396 | done: |
| 372 | − bpf_link__destroy(egress_link); | |
| 373 | − bpf_link__destroy(ingress_link); | |
| 397 | + test_sock_fields__detach(skel); | |
| 374 | 398 | test_sock_fields__destroy(skel); |
| 375 | 399 | if (child_cg_fd >= 0) |
| 376 | 400 | close(child_cg_fd); |
| @@ -12,6 +12,7 @@ · 11 unmodified lines | ||
| 12 | 12 | enum bpf_linum_array_idx { |
| 13 | 13 | EGRESS_LINUM_IDX, |
| 14 | 14 | INGRESS_LINUM_IDX, |
| 15 | + READ_SK_DST_PORT_LINUM_IDX, | |
| 15 | 16 | __NR_BPF_LINUM_ARRAY_IDX, |
| 16 | 17 | }; |
| 17 | 18 | |
| @@ -250,4 +251,44 @@ int ingress_read_sock_fields(struct __sk_buff *skb) · 232 unmodified lines | ||
| 250 | 251 | return CG_OK; |
| 251 | 252 | } |
| 252 | 253 | |
| 254 | +static __noinline bool sk_dst_port__load_word(struct bpf_sock *sk) | |
| 255 | +{ | |
| 256 | + __u32 *word = (__u32 *)&sk->dst_port; | |
| 257 | + return word[0] == bpf_htonl(0xcafe0000); | |
| 258 | +} | |
| 259 | + | |
| 260 | +static __noinline bool sk_dst_port__load_half(struct bpf_sock *sk) | |
| 261 | +{ | |
| 262 | + __u16 *half = (__u16 *)&sk->dst_port; | |
| 263 | + return half[0] == bpf_htons(0xcafe); | |
| 264 | +} | |
| 265 | + | |
| 266 | +static __noinline bool sk_dst_port__load_byte(struct bpf_sock *sk) | |
| 267 | +{ | |
| 268 | + __u8 *byte = (__u8 *)&sk->dst_port; | |
| 269 | + return byte[0] == 0xca && byte[1] == 0xfe; | |
| 270 | +} | |
| 271 | + | |
| 272 | +SEC("cgroup_skb/egress") | |
| 273 | +int read_sk_dst_port(struct __sk_buff *skb) | |
| 274 | +{ | |
| 275 | + __u32 linum, linum_idx; | |
| 276 | + struct bpf_sock *sk; | |
| 277 | + | |
| 278 | + linum_idx = READ_SK_DST_PORT_LINUM_IDX; | |
| 279 | + | |
| 280 | + sk = skb->sk; | |
| 281 | + if (!sk) | |
| 282 | + RET_LOG(); | |
| 283 | + | |
| 284 | + if (!sk_dst_port__load_word(sk)) | |
| 285 | + RET_LOG(); | |
| 286 | + if (!sk_dst_port__load_half(sk)) | |
| 287 | + RET_LOG(); | |
| 288 | + if (!sk_dst_port__load_byte(sk)) | |
| 289 | + RET_LOG(); | |
| 290 | + | |
| 291 | + return CG_OK; | |
| 292 | +} | |
| 293 | + | |
| 253 | 294 | char _license[] SEC("license") = "GPL"; |
| @@ -121,7 +121,25 @@ · 120 unmodified lines | ||
| 121 | 121 | .result = ACCEPT, |
| 122 | 122 | }, |
| 123 | 123 | { |
| 124 | − "sk_fullsock(skb->sk): sk->dst_port [narrow load]", | |
| 124 | + "sk_fullsock(skb->sk): sk->dst_port [word load] (backward compatibility)", | |
| 125 | + .insns = { | |
| 126 | + BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)), | |
| 127 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2), | |
| 128 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 129 | + BPF_EXIT_INSN(), | |
| 130 | + BPF_EMIT_CALL(BPF_FUNC_sk_fullsock), | |
| 131 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2), | |
| 132 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 133 | + BPF_EXIT_INSN(), | |
| 134 | + BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_0, offsetof(struct bpf_sock, dst_port)), | |
| 135 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 136 | + BPF_EXIT_INSN(), | |
| 137 | + }, | |
| 138 | + .prog_type = BPF_PROG_TYPE_CGROUP_SKB, | |
| 139 | + .result = ACCEPT, | |
| 140 | +}, | |
| 141 | +{ | |
| 142 | + "sk_fullsock(skb->sk): sk->dst_port [half load]", | |
| 125 | 143 | .insns = { |
| 126 | 144 | BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)), |
| 127 | 145 | BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2), |
| @@ -139,7 +157,64 @@ · 11 unmodified lines | ||
| 139 | 157 | .result = ACCEPT, |
| 140 | 158 | }, |
| 141 | 159 | { |
| 142 | − "sk_fullsock(skb->sk): sk->dst_port [load 2nd byte]", | |
| 160 | + "sk_fullsock(skb->sk): sk->dst_port [half load] (invalid)", | |
| 161 | + .insns = { | |
| 162 | + BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)), | |
| 163 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2), | |
| 164 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 165 | + BPF_EXIT_INSN(), | |
| 166 | + BPF_EMIT_CALL(BPF_FUNC_sk_fullsock), | |
| 167 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2), | |
| 168 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 169 | + BPF_EXIT_INSN(), | |
| 170 | + BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_0, offsetof(struct bpf_sock, dst_port) + 2), | |
| 171 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 172 | + BPF_EXIT_INSN(), | |
| 173 | + }, | |
| 174 | + .prog_type = BPF_PROG_TYPE_CGROUP_SKB, | |
| 175 | + .result = REJECT, | |
| 176 | + .errstr = "invalid sock access", | |
| 177 | +}, | |
| 178 | +{ | |
| 179 | + "sk_fullsock(skb->sk): sk->dst_port [byte load]", | |
| 180 | + .insns = { | |
| 181 | + BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)), | |
| 182 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2), | |
| 183 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 184 | + BPF_EXIT_INSN(), | |
| 185 | + BPF_EMIT_CALL(BPF_FUNC_sk_fullsock), | |
| 186 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2), | |
| 187 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 188 | + BPF_EXIT_INSN(), | |
| 189 | + BPF_LDX_MEM(BPF_B, BPF_REG_2, BPF_REG_0, offsetof(struct bpf_sock, dst_port)), | |
| 190 | + BPF_LDX_MEM(BPF_B, BPF_REG_2, BPF_REG_0, offsetof(struct bpf_sock, dst_port) + 1), | |
| 191 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 192 | + BPF_EXIT_INSN(), | |
| 193 | + }, | |
| 194 | + .prog_type = BPF_PROG_TYPE_CGROUP_SKB, | |
| 195 | + .result = ACCEPT, | |
| 196 | +}, | |
| 197 | +{ | |
| 198 | + "sk_fullsock(skb->sk): sk->dst_port [byte load] (invalid)", | |
| 199 | + .insns = { | |
| 200 | + BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)), | |
| 201 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2), | |
| 202 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 203 | + BPF_EXIT_INSN(), | |
| 204 | + BPF_EMIT_CALL(BPF_FUNC_sk_fullsock), | |
| 205 | + BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2), | |
| 206 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 207 | + BPF_EXIT_INSN(), | |
| 208 | + BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_0, offsetof(struct bpf_sock, dst_port) + 2), | |
| 209 | + BPF_MOV64_IMM(BPF_REG_0, 0), | |
| 210 | + BPF_EXIT_INSN(), | |
| 211 | + }, | |
| 212 | + .prog_type = BPF_PROG_TYPE_CGROUP_SKB, | |
| 213 | + .result = REJECT, | |
| 214 | + .errstr = "invalid sock access", | |
| 215 | +}, | |
| 216 | +{ | |
| 217 | + "sk_fullsock(skb->sk): past sk->dst_port [half load] (invalid)", | |
| 143 | 218 | .insns = { |
| 144 | 219 | BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, offsetof(struct __sk_buff, sk)), |
| 145 | 220 | BPF_JMP_IMM(BPF_JNE, BPF_REG_1, 0, 2), |
| @@ -149,7 +224,7 @@ · 3 unmodified lines | ||
| 149 | 224 | BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2), |
| 150 | 225 | BPF_MOV64_IMM(BPF_REG_0, 0), |
| 151 | 226 | BPF_EXIT_INSN(), |
| 152 | − BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_0, offsetof(struct bpf_sock, dst_port) + 1), | |
| 227 | + BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_0, offsetofend(struct bpf_sock, dst_port)), | |
| 153 | 228 | BPF_MOV64_IMM(BPF_REG_0, 0), |
| 154 | 229 | BPF_EXIT_INSN(), |
| 155 | 230 | }, |