bpf: Reject narrower access to pointer ctx fields

The following BPF program, simplified from a syzkaller repro, causes a
kernel warning:

    r0 = *(u8 *)(r1 + 169);
    exit;

With pointer field sk being at offset 168 in __sk_buff. This access is
detected as a narrower read in bpf_skb_is_valid_access because it
doesn't match offsetof(struct __sk_buff, sk). It is therefore allowed
and later proceeds to bpf_convert_ctx_access. Note that for the
"is_narrower_load" case in the convert_ctx_accesses(), the insn->off
is aligned, so the cnt may not be 0 because it matches the
offsetof(struct __sk_buff, sk) in the bpf_convert_ctx_access. However,
the target_size stays 0 and the verifier errors with a kernel warning:

    verifier bug: error during ctx access conversion(1)

This patch fixes that to return a proper "invalid bpf_context access
off=X size=Y" error on the load instruction.

The same issue affects multiple other fields in context structures that
allow narrow access. Some other non-affected fields (for sk_msg,
sk_lookup, and sockopt) were also changed to use bpf_ctx_range_ptr for
consistency.

Note this syzkaller crash was reported in the "Closes" link below, which
used to be about a different bug, fixed in
commit fce7bd8e385a ("bpf/verifier: Handle BPF_LOAD_ACQ instructions
in insn_def_regno()"). Because syzbot somehow confused the two bugs,
the new crash and repro didn't get reported to the mailing list.

Fixes: f96da09473b52 ("bpf: simplify narrower ctx access")
Fixes: 0df1a55afa832 ("bpf: Warn on internal verifier errors")
Reported-by: syzbot+0ef84a7bdf5301d4cbec@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0ef84a7bdf5301d4cbec
Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Link: https://patch.msgid.link/3b8dcee67ff4296903351a974ddd9c4dca768b64.1753194596.git.paul.chaignon@gmail.com

diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index f4885514f007b3da34e505aad899f5f2ec4eaba2..deb88fade2499e2f556d4db9990d9fc6a107301a 100644 (file)
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -2440,22 +2440,22 @@ static bool cg_sockopt_is_valid_access(int off, int size,
        }
 
        switch (off) {
-       case offsetof(struct bpf_sockopt, sk):
+       case bpf_ctx_range_ptr(struct bpf_sockopt, sk):
                if (size != sizeof(__u64))
                        return false;
                info->reg_type = PTR_TO_SOCKET;
                break;
-       case offsetof(struct bpf_sockopt, optval):
+       case bpf_ctx_range_ptr(struct bpf_sockopt, optval):
                if (size != sizeof(__u64))
                        return false;
                info->reg_type = PTR_TO_PACKET;
                break;
-       case offsetof(struct bpf_sockopt, optval_end):
+       case bpf_ctx_range_ptr(struct bpf_sockopt, optval_end):
                if (size != sizeof(__u64))
                        return false;
                info->reg_type = PTR_TO_PACKET_END;
                break;
-       case offsetof(struct bpf_sockopt, retval):
+       case bpf_ctx_range(struct bpf_sockopt, retval):
                if (size != size_default)
                        return false;
                return prog->expected_attach_type == BPF_CGROUP_GETSOCKOPT;

diff --git a/net/core/filter.c b/net/core/filter.c
index 2eb8947d80976c3f4e8ed1d2b0b7ee688973cca1..c09a85c17496e1c3e38a61ddb6c657c6853eb5c4 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -8699,7 +8699,7 @@ static bool bpf_skb_is_valid_access(int off, int size, enum bpf_access_type type
                if (size != sizeof(__u64))
                        return false;
                break;
-       case offsetof(struct __sk_buff, sk):
+       case bpf_ctx_range_ptr(struct __sk_buff, sk):
                if (type == BPF_WRITE || size != sizeof(__u64))
                        return false;
                info->reg_type = PTR_TO_SOCK_COMMON_OR_NULL;
@@ -9277,7 +9277,7 @@ static bool sock_addr_is_valid_access(int off, int size,
                                return false;
                }
                break;
-       case offsetof(struct bpf_sock_addr, sk):
+       case bpf_ctx_range_ptr(struct bpf_sock_addr, sk):
                if (type != BPF_READ)
                        return false;
                if (size != sizeof(__u64))
@@ -9327,17 +9327,17 @@ static bool sock_ops_is_valid_access(int off, int size,
                        if (size != sizeof(__u64))
                                return false;
                        break;
-               case offsetof(struct bpf_sock_ops, sk):
+               case bpf_ctx_range_ptr(struct bpf_sock_ops, sk):
                        if (size != sizeof(__u64))
                                return false;
                        info->reg_type = PTR_TO_SOCKET_OR_NULL;
                        break;
-               case offsetof(struct bpf_sock_ops, skb_data):
+               case bpf_ctx_range_ptr(struct bpf_sock_ops, skb_data):
                        if (size != sizeof(__u64))
                                return false;
                        info->reg_type = PTR_TO_PACKET;
                        break;
-               case offsetof(struct bpf_sock_ops, skb_data_end):
+               case bpf_ctx_range_ptr(struct bpf_sock_ops, skb_data_end):
                        if (size != sizeof(__u64))
                                return false;
                        info->reg_type = PTR_TO_PACKET_END;
@@ -9346,7 +9346,7 @@ static bool sock_ops_is_valid_access(int off, int size,
                        bpf_ctx_record_field_size(info, size_default);
                        return bpf_ctx_narrow_access_ok(off, size,
                                                        size_default);
-               case offsetof(struct bpf_sock_ops, skb_hwtstamp):
+               case bpf_ctx_range(struct bpf_sock_ops, skb_hwtstamp):
                        if (size != sizeof(__u64))
                                return false;
                        break;
@@ -9416,17 +9416,17 @@ static bool sk_msg_is_valid_access(int off, int size,
                return false;
 
        switch (off) {
-       case offsetof(struct sk_msg_md, data):
+       case bpf_ctx_range_ptr(struct sk_msg_md, data):
                info->reg_type = PTR_TO_PACKET;
                if (size != sizeof(__u64))
                        return false;
                break;
-       case offsetof(struct sk_msg_md, data_end):
+       case bpf_ctx_range_ptr(struct sk_msg_md, data_end):
                info->reg_type = PTR_TO_PACKET_END;
                if (size != sizeof(__u64))
                        return false;
                break;
-       case offsetof(struct sk_msg_md, sk):
+       case bpf_ctx_range_ptr(struct sk_msg_md, sk):
                if (size != sizeof(__u64))
                        return false;
                info->reg_type = PTR_TO_SOCKET;
@@ -11632,7 +11632,7 @@ static bool sk_lookup_is_valid_access(int off, int size,
                return false;
 
        switch (off) {
-       case offsetof(struct bpf_sk_lookup, sk):
+       case bpf_ctx_range_ptr(struct bpf_sk_lookup, sk):
                info->reg_type = PTR_TO_SOCKET_OR_NULL;
                return size == sizeof(__u64);