Commit 46e9244b authored by Alexei Starovoitov's avatar Alexei Starovoitov
Browse files

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/20220222182559.2865596-2-iii@linux.ibm.com/

v1 -> v2:
- Remove needless check that 4-byte load is from &ctx->remote_port offset
  (Martin)

[v1]: https://lore.kernel.org/bpf/20220317165826.1099418-1-jakub@cloudflare.com/


====================

Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents 30630e44 ce523680

net/core/filter.c

+18 −2
Original line number Diff line number Diff line
@@ -10989,13 +10989,24 @@ static bool sk_lookup_is_valid_access(int off, int size, 
	case bpf_ctx_range(struct bpf_sk_lookup, local_ip4):

	case bpf_ctx_range_till(struct bpf_sk_lookup, remote_ip6[0], remote_ip6[3]):

	case bpf_ctx_range_till(struct bpf_sk_lookup, local_ip6[0], local_ip6[3]):

	case offsetof(struct bpf_sk_lookup, remote_port) ...

	     offsetof(struct bpf_sk_lookup, local_ip4) - 1:

	case bpf_ctx_range(struct bpf_sk_lookup, local_port):

	case bpf_ctx_range(struct bpf_sk_lookup, ingress_ifindex):

		bpf_ctx_record_field_size(info, sizeof(__u32));

		return bpf_ctx_narrow_access_ok(off, size, sizeof(__u32));



	case bpf_ctx_range(struct bpf_sk_lookup, remote_port):

		/* Allow 4-byte access to 2-byte field for backward compatibility */

		if (size == sizeof(__u32))

			return true;

		bpf_ctx_record_field_size(info, sizeof(__be16));

		return bpf_ctx_narrow_access_ok(off, size, sizeof(__be16));



	case offsetofend(struct bpf_sk_lookup, remote_port) ...

	     offsetof(struct bpf_sk_lookup, local_ip4) - 1:

		/* Allow access to zero padding for backward compatibility */

		bpf_ctx_record_field_size(info, sizeof(__u16));

		return bpf_ctx_narrow_access_ok(off, size, sizeof(__u16));



	default:

		return false;

	}
@@ -11077,6 +11088,11 @@ static u32 sk_lookup_convert_ctx_access(enum bpf_access_type type, 
						     sport, 2, target_size));

		break;



	case offsetofend(struct bpf_sk_lookup, remote_port):

		*target_size = 2;

		*insn++ = BPF_MOV32_IMM(si->dst_reg, 0);

		break;



	case offsetof(struct bpf_sk_lookup, local_port):

		*insn++ = BPF_LDX_MEM(BPF_H, si->dst_reg, si->src_reg,

				      bpf_target_off(struct bpf_sk_lookup_kern,

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

+9 −4
Original line number Diff line number Diff line
@@ -413,15 +413,20 @@ int ctx_narrow_access(struct bpf_sk_lookup *ctx) 


	/* Narrow loads from remote_port field. Expect SRC_PORT. */

	if (LSB(ctx->remote_port, 0) != ((SRC_PORT >> 0) & 0xff) ||

	    LSB(ctx->remote_port, 1) != ((SRC_PORT >> 8) & 0xff) ||

	    LSB(ctx->remote_port, 2) != 0 || LSB(ctx->remote_port, 3) != 0)

	    LSB(ctx->remote_port, 1) != ((SRC_PORT >> 8) & 0xff))

		return SK_DROP;

	if (LSW(ctx->remote_port, 0) != SRC_PORT)

		return SK_DROP;



	/* Load from remote_port field with zero padding (backward compatibility) */

	/*

	 * NOTE: 4-byte load from bpf_sk_lookup at remote_port offset

	 * is quirky. It gets rewritten by the access converter to a

	 * 2-byte load for backward compatibility. Treating the load

	 * result as a be16 value makes the code portable across

	 * little- and big-endian platforms.

	 */

	val_u32 = *(__u32 *)&ctx->remote_port;

	if (val_u32 != bpf_htonl(bpf_ntohs(SRC_PORT) << 16))

	if (val_u32 != SRC_PORT)

		return SK_DROP;



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