bpf: Make dst_port field in struct bpf_sock 16-bit wide

[ 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>

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index e3fb5e52051127af606166cfe321f4f1cd558997..2136e45656ab2b7fcf1406c6d3834b477d6d5912 100644 (file)
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5347,7 +5347,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 76e406965b6f98a9f3d98f156ae16ed23a8874a2..a65de7ac60aa9c18e2ac244c359b925d41cbb764 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -7966,6 +7966,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;
@@ -7977,7 +7978,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):
@@ -7986,6 +7986,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;