Skip to content
Snippets Groups Projects
Commit 1d29b1e6 authored by Augusto Caringi's avatar Augusto Caringi
Browse files

Merge: [RHEL 9.7] NULL pointer dereference in bpf_sk_storage_tracing_allowed()...

Merge: [RHEL 9.7] NULL pointer dereference in bpf_sk_storage_tracing_allowed() when attaching BPF program to non-vmlinux BTF

MR: https://gitlab.com/redhat/centos-stream/src/kernel/centos-stream-9/-/merge_requests/6524

JIRA: https://issues.redhat.com/browse/RHEL-82439



```
commit 7332537962956fab2c055b37e5e2e6a0d2a8d6bf
Author: Jared Kangas <jkangas@redhat.com>
Date:   Tue Jan 21 06:25:04 2025 -0800

    bpf: Remove unnecessary BTF lookups in bpf_sk_storage_tracing_allowed

    When loading BPF programs, bpf_sk_storage_tracing_allowed() does a
    series of lookups to get a type name from the program's attach_btf_id,
    making the assumption that the type is present in the vmlinux BTF along
    the way. However, this results in btf_type_by_id() returning a null
    pointer if a non-vmlinux kernel BTF is attached to. Proof-of-concept on
    a kernel with CONFIG_IPV6=m:

        $ cat bpfcrash.c
        #include <unistd.h>
        #include <linux/bpf.h>
        #include <sys/syscall.h>

        static int bpf(enum bpf_cmd cmd, union bpf_attr *attr)
        {
            return syscall(__NR_bpf, cmd, attr, sizeof(*attr));
        }

        int main(void)
        {
            const int btf_fd = bpf(BPF_BTF_GET_FD_BY_ID, &(union bpf_attr) {
                .btf_id = BTF_ID,
            });
            if (btf_fd < 0)
                return 1;

            const int bpf_sk_storage_get = 107;
            const struct bpf_insn insns[] = {
                { .code = BPF_JMP | BPF_CALL, .imm = bpf_sk_storage_get},
                { .code = BPF_JMP | BPF_EXIT },
            };
            return bpf(BPF_PROG_LOAD, &(union bpf_attr) {
                .prog_type            = BPF_PROG_TYPE_TRACING,
                .expected_attach_type = BPF_TRACE_FENTRY,
                .license              = (unsigned long)"GPL",
                .insns                = (unsigned long)&insns,
                .insn_cnt             = sizeof(insns) / sizeof(insns[0]),
                .attach_btf_obj_fd    = btf_fd,
                .attach_btf_id        = TYPE_ID,
            });
        }
        $ sudo bpftool btf list | grep ipv6
        2: name [ipv6]  size 928200B
        $ sudo bpftool btf dump id 2 | awk '$3 ~ /inet6_sock_destruct/'
        [130689] FUNC 'inet6_sock_destruct' type_id=130677 linkage=static
        $ gcc -D_DEFAULT_SOURCE -DBTF_ID=2 -DTYPE_ID=130689 \
            bpfcrash.c -o bpfcrash
        $ sudo ./bpfcrash

    This causes a null pointer dereference:

        Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
        Call trace:
         bpf_sk_storage_tracing_allowed+0x8c/0xb0 P
         check_helper_call.isra.0+0xa8/0x1730
         do_check+0xa18/0xb40
         do_check_common+0x140/0x640
         bpf_check+0xb74/0xcb8
         bpf_prog_load+0x598/0x9a8
         __sys_bpf+0x580/0x980
         __arm64_sys_bpf+0x28/0x40
         invoke_syscall.constprop.0+0x54/0xe8
         do_el0_svc+0xb4/0xd0
         el0_svc+0x44/0x1f8
         el0t_64_sync_handler+0x13c/0x160
         el0t_64_sync+0x184/0x188

    Resolve this by using prog->aux->attach_func_name and removing the
    lookups.

    Fixes: 8e4597c6 ("bpf: Allow using bpf_sk_storage in FENTRY/FEXIT/RAW_TP")
Suggested-by: default avatarMartin KaFai Lau <martin.lau@linux.dev>
Signed-off-by: default avatarJared Kangas <jkangas@redhat.com>
Signed-off-by: default avatarMartin KaFai Lau <martin.lau@kernel.org>
    Link: https://patch.msgid.link/20250121142504.1369436-1-jkangas@redhat.com


Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org&gt;```>

Signed-off-by: default avatarCKI Backport Bot <cki-ci-bot+cki-gitlab-backport-bot@redhat.com>

---

<small>Created 2025-03-06 16:33 UTC by backporter - [KWF FAQ](https://red.ht/kernel_workflow_doc) - [Slack #team-kernel-workflow](https://redhat-internal.slack.com/archives/C04LRUPMJQ5) - [Source](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/webhook/utils/backporter.py) - [Documentation](https://gitlab.com/cki-project/kernel-workflow/-/blob/main/docs/README.backporter.md) - [Report an issue](https://gitlab.com/cki-project/kernel-workflow/-/issues/new?issue%5Btitle%5D=backporter%20webhook%20issue)</small

>

Approved-by: default avatarAntoine Tenart <atenart@redhat.com>
Approved-by: default avatarRadu Rendec <rrendec@redhat.com>
Approved-by: default avatarCKI KWF Bot <cki-ci-bot+kwf-gitlab-com@redhat.com>

Merged-by: default avatarAugusto Caringi <acaringi@redhat.com>
parents 67469b87 f89a2d8a
No related branches found
No related tags found
No related merge requests found
......@@ -351,11 +351,6 @@ const struct bpf_func_proto bpf_sk_storage_delete_proto = {
static bool bpf_sk_storage_tracing_allowed(const struct bpf_prog *prog)
{
const struct btf *btf_vmlinux;
const struct btf_type *t;
const char *tname;
u32 btf_id;
if (prog->aux->dst_prog)
return false;
......@@ -370,13 +365,7 @@ static bool bpf_sk_storage_tracing_allowed(const struct bpf_prog *prog)
return true;
case BPF_TRACE_FENTRY:
case BPF_TRACE_FEXIT:
btf_vmlinux = bpf_get_btf_vmlinux();
if (IS_ERR_OR_NULL(btf_vmlinux))
return false;
btf_id = prog->aux->attach_btf_id;
t = btf_type_by_id(btf_vmlinux, btf_id);
tname = btf_name_by_offset(btf_vmlinux, t->name_off);
return !!strncmp(tname, "bpf_sk_storage",
return !!strncmp(prog->aux->attach_func_name, "bpf_sk_storage",
strlen("bpf_sk_storage"));
default:
return false;
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment