Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpf: Fix a bpf_kptr_xchg() issue with local kptr #5549

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -7745,7 +7745,18 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
verbose(env, "verifier internal error: unimplemented handling of MEM_ALLOC\n");
return -EFAULT;
}
/* Handled by helper specific checks */
if (meta->func_id == BPF_FUNC_kptr_xchg) {
struct btf_field *kptr_field = meta->kptr_field;

if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, reg->off,
kptr_field->kptr.btf, kptr_field->kptr.btf_id,
true)) {
verbose(env, "R%d is of type %s but %s is expected\n",
regno, btf_type_name(reg->btf, reg->btf_id),
btf_type_name(kptr_field->kptr.btf, kptr_field->kptr.btf_id));
return -EACCES;
}
}
break;
case PTR_TO_BTF_ID | MEM_PERCPU:
case PTR_TO_BTF_ID | MEM_PERCPU | PTR_TRUSTED:
Expand Down
10 changes: 9 additions & 1 deletion tools/testing/selftests/bpf/prog_tests/local_kptr_stash.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <network_helpers.h>

#include "local_kptr_stash.skel.h"
#include "local_kptr_stash_fail.skel.h"
static void test_local_kptr_stash_simple(void)
{
LIBBPF_OPTS(bpf_test_run_opts, opts,
Expand Down Expand Up @@ -51,10 +52,17 @@ static void test_local_kptr_stash_unstash(void)
local_kptr_stash__destroy(skel);
}

void test_local_kptr_stash_success(void)
static void test_local_kptr_stash_fail(void)
{
RUN_TESTS(local_kptr_stash_fail);
}

void test_local_kptr_stash(void)
{
if (test__start_subtest("local_kptr_stash_simple"))
test_local_kptr_stash_simple();
if (test__start_subtest("local_kptr_stash_unstash"))
test_local_kptr_stash_unstash();
if (test__start_subtest("local_kptr_stash_fail"))
test_local_kptr_stash_fail();
}
65 changes: 65 additions & 0 deletions tools/testing/selftests/bpf/progs/local_kptr_stash_fail.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// SPDX-License-Identifier: GPL-2.0
/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */

#include <vmlinux.h>
#include <bpf/bpf_helpers.h>
#include <bpf/bpf_tracing.h>
#include <bpf/bpf_core_read.h>
#include "../bpf_experimental.h"
#include "bpf_misc.h"

struct node_data {
long key;
long data;
struct bpf_rb_node node;
};

struct map_value {
struct node_data __kptr *node;
};

struct node_data2 {
long key[4];
};

/* This is necessary so that LLVM generates BTF for node_data struct
* If it's not included, a fwd reference for node_data will be generated but
* no struct. Example BTF of "node" field in map_value when not included:
*
* [10] PTR '(anon)' type_id=35
* [34] FWD 'node_data' fwd_kind=struct
* [35] TYPE_TAG 'kptr_ref' type_id=34
*/
struct node_data *just_here_because_btf_bug;

struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__type(key, int);
__type(value, struct map_value);
__uint(max_entries, 2);
} some_nodes SEC(".maps");

SEC("tc")
__failure __msg("R2 is of type node_data2 but node_data is expected")
long stash_rb_nodes(void *ctx)
{
struct map_value *mapval;
struct node_data2 *res;
int idx = 0;

mapval = bpf_map_lookup_elem(&some_nodes, &idx);
if (!mapval)
return 1;

res = bpf_obj_new(typeof(*res));
if (!res)
return 1;
res->key[0] = 40;

res = bpf_kptr_xchg(&mapval->node, res);
if (res)
bpf_obj_drop(res);
return 0;
}

char _license[] SEC("license") = "GPL";
Loading