Skip to content

Commit 46e9244

Browse files
author
Alexei Starovoitov
committed
Merge branch 'Make 2-byte access to bpf_sk_lookup->remote_port endian-agnostic'
Jakub Sitnicki says: ==================== This patch set is a result of a discussion we had around the RFC patchset from Ilya [1]. The fix for the narrow loads from the RFC series is still relevant, but this series does not depend on it. Nor is it required to unbreak sk_lookup tests on BE, if this series gets applied. To summarize the takeaways from [1]: 1) we want to make 2-byte load from ctx->remote_port portable across LE and BE, 2) we keep the 4-byte load from ctx->remote_port as it is today - result varies on endianess of the platform. [1] https://lore.kernel.org/bpf/[email protected]/ v1 -> v2: - Remove needless check that 4-byte load is from &ctx->remote_port offset (Martin) [v1]: https://lore.kernel.org/bpf/[email protected]/ ==================== Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents 30630e4 + ce52368 commit 46e9244

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

net/core/filter.c

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10989,13 +10989,24 @@ static bool sk_lookup_is_valid_access(int off, int size,
1098910989
case bpf_ctx_range(struct bpf_sk_lookup, local_ip4):
1099010990
case bpf_ctx_range_till(struct bpf_sk_lookup, remote_ip6[0], remote_ip6[3]):
1099110991
case bpf_ctx_range_till(struct bpf_sk_lookup, local_ip6[0], local_ip6[3]):
10992-
case offsetof(struct bpf_sk_lookup, remote_port) ...
10993-
offsetof(struct bpf_sk_lookup, local_ip4) - 1:
1099410992
case bpf_ctx_range(struct bpf_sk_lookup, local_port):
1099510993
case bpf_ctx_range(struct bpf_sk_lookup, ingress_ifindex):
1099610994
bpf_ctx_record_field_size(info, sizeof(__u32));
1099710995
return bpf_ctx_narrow_access_ok(off, size, sizeof(__u32));
1099810996

10997+
case bpf_ctx_range(struct bpf_sk_lookup, remote_port):
10998+
/* Allow 4-byte access to 2-byte field for backward compatibility */
10999+
if (size == sizeof(__u32))
11000+
return true;
11001+
bpf_ctx_record_field_size(info, sizeof(__be16));
11002+
return bpf_ctx_narrow_access_ok(off, size, sizeof(__be16));
11003+
11004+
case offsetofend(struct bpf_sk_lookup, remote_port) ...
11005+
offsetof(struct bpf_sk_lookup, local_ip4) - 1:
11006+
/* Allow access to zero padding for backward compatibility */
11007+
bpf_ctx_record_field_size(info, sizeof(__u16));
11008+
return bpf_ctx_narrow_access_ok(off, size, sizeof(__u16));
11009+
1099911010
default:
1100011011
return false;
1100111012
}
@@ -11077,6 +11088,11 @@ static u32 sk_lookup_convert_ctx_access(enum bpf_access_type type,
1107711088
sport, 2, target_size));
1107811089
break;
1107911090

11091+
case offsetofend(struct bpf_sk_lookup, remote_port):
11092+
*target_size = 2;
11093+
*insn++ = BPF_MOV32_IMM(si->dst_reg, 0);
11094+
break;
11095+
1108011096
case offsetof(struct bpf_sk_lookup, local_port):
1108111097
*insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->src_reg,
1108211098
bpf_target_off(struct bpf_sk_lookup_kern,

tools/testing/selftests/bpf/progs/test_sk_lookup.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -413,15 +413,20 @@ int ctx_narrow_access(struct bpf_sk_lookup *ctx)
413413

414414
/* Narrow loads from remote_port field. Expect SRC_PORT. */
415415
if (LSB(ctx->remote_port, 0) != ((SRC_PORT >> 0) & 0xff) ||
416-
LSB(ctx->remote_port, 1) != ((SRC_PORT >> 8) & 0xff) ||
417-
LSB(ctx->remote_port, 2) != 0 || LSB(ctx->remote_port, 3) != 0)
416+
LSB(ctx->remote_port, 1) != ((SRC_PORT >> 8) & 0xff))
418417
return SK_DROP;
419418
if (LSW(ctx->remote_port, 0) != SRC_PORT)
420419
return SK_DROP;
421420

422-
/* Load from remote_port field with zero padding (backward compatibility) */
421+
/*
422+
* NOTE: 4-byte load from bpf_sk_lookup at remote_port offset
423+
* is quirky. It gets rewritten by the access converter to a
424+
* 2-byte load for backward compatibility. Treating the load
425+
* result as a be16 value makes the code portable across
426+
* little- and big-endian platforms.
427+
*/
423428
val_u32 = *(__u32 *)&ctx->remote_port;
424-
if (val_u32 != bpf_htonl(bpf_ntohs(SRC_PORT) << 16))
429+
if (val_u32 != SRC_PORT)
425430
return SK_DROP;
426431

427432
/* Narrow loads from local_port field. Expect DST_PORT. */

0 commit comments

Comments
 (0)