| From 41b8aa2d4046b7a23faae6360be185785d6c376d Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Sun, 30 Jan 2022 12:55:17 +0100 |
| Subject: bpf: Make dst_port field in struct bpf_sock 16-bit wide |
| |
| From: Jakub Sitnicki <jakub@cloudflare.com> |
| |
| [ Upstream commit 4421a582718ab81608d8486734c18083b822390d ] |
| |
| Menglong Dong reports that the documentation for the dst_port field in |
| struct bpf_sock is inaccurate and confusing. From the BPF program PoV, the |
| field is a zero-padded 16-bit integer in network byte order. The value |
| appears to the BPF user as if laid out in memory as so: |
| |
| offsetof(struct bpf_sock, dst_port) + 0 <port MSB> |
| + 8 <port LSB> |
| +16 0x00 |
| +24 0x00 |
| |
| 32-, 16-, and 8-bit wide loads from the field are all allowed, but only if |
| the offset into the field is 0. |
| |
| 32-bit wide loads from dst_port are especially confusing. The loaded value, |
| after converting to host byte order with bpf_ntohl(dst_port), contains the |
| port number in the upper 16-bits. |
| |
| Remove the confusion by splitting the field into two 16-bit fields. For |
| backward compatibility, allow 32-bit wide loads from offsetof(struct |
| bpf_sock, dst_port). |
| |
| While at it, allow loads 8-bit loads at offset [0] and [1] from dst_port. |
| |
| Reported-by: Menglong Dong <imagedong@tencent.com> |
| Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> |
| Link: https://lore.kernel.org/r/20220130115518.213259-2-jakub@cloudflare.com |
| Signed-off-by: Alexei Starovoitov <ast@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| include/uapi/linux/bpf.h | 3 ++- |
| net/core/filter.c | 10 +++++++++- |
| 2 files changed, 11 insertions(+), 2 deletions(-) |
| |
| diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h |
| index 2b3c3f83076c..61bbaf340a89 100644 |
| --- a/include/uapi/linux/bpf.h |
| +++ b/include/uapi/linux/bpf.h |
| @@ -5414,7 +5414,8 @@ struct bpf_sock { |
| __u32 src_ip4; |
| __u32 src_ip6[4]; |
| __u32 src_port; /* host byte order */ |
| - __u32 dst_port; /* network byte order */ |
| + __be16 dst_port; /* network byte order */ |
| + __u16 :16; /* zero padding */ |
| __u32 dst_ip4; |
| __u32 dst_ip6[4]; |
| __u32 state; |
| diff --git a/net/core/filter.c b/net/core/filter.c |
| index d4cdf11656b3..4721ed65bcc5 100644 |
| --- a/net/core/filter.c |
| +++ b/net/core/filter.c |
| @@ -7975,6 +7975,7 @@ bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type, |
| struct bpf_insn_access_aux *info) |
| { |
| const int size_default = sizeof(__u32); |
| + int field_size; |
| |
| if (off < 0 || off >= sizeof(struct bpf_sock)) |
| return false; |
| @@ -7986,7 +7987,6 @@ bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type, |
| case offsetof(struct bpf_sock, family): |
| case offsetof(struct bpf_sock, type): |
| case offsetof(struct bpf_sock, protocol): |
| - case offsetof(struct bpf_sock, dst_port): |
| case offsetof(struct bpf_sock, src_port): |
| case offsetof(struct bpf_sock, rx_queue_mapping): |
| case bpf_ctx_range(struct bpf_sock, src_ip4): |
| @@ -7995,6 +7995,14 @@ bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type, |
| case bpf_ctx_range_till(struct bpf_sock, dst_ip6[0], dst_ip6[3]): |
| bpf_ctx_record_field_size(info, size_default); |
| return bpf_ctx_narrow_access_ok(off, size, size_default); |
| + case bpf_ctx_range(struct bpf_sock, dst_port): |
| + field_size = size == size_default ? |
| + size_default : sizeof_field(struct bpf_sock, dst_port); |
| + bpf_ctx_record_field_size(info, field_size); |
| + return bpf_ctx_narrow_access_ok(off, size, field_size); |
| + case offsetofend(struct bpf_sock, dst_port) ... |
| + offsetof(struct bpf_sock, dst_ip4) - 1: |
| + return false; |
| } |
| |
| return size == size_default; |
| -- |
| 2.35.1 |
| |