Commit 5e1d40b7 authored by Alexei Starovoitov's avatar Alexei Starovoitov

Merge branch 'Add support of pointer to struct in global'

Dmitrii Banshchikov says:

====================

This patchset adds support of pointers to type with known size among
global function arguments.

The motivation is to overcome the limit on the maximum number of allowed
arguments and avoid tricky and unoptimal ways of passing arguments.

A referenced type may contain pointers but access via such pointers
cannot be veirified currently.

v2 -> v3
 - Fix reg ID generation
 - Fix commit description
 - Fix typo
 - Fix tests

v1 -> v2:
 - Allow pointer to any type with known size rather than struct only
 - Allow pointer in global functions only
 - Add more tests
 - Fix wrapping and v1 comments
====================
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parents b62eba56 8b08807d
......@@ -471,6 +471,8 @@ bpf_prog_offload_remove_insns(struct bpf_verifier_env *env, u32 off, u32 cnt);
int check_ctx_reg(struct bpf_verifier_env *env,
const struct bpf_reg_state *reg, int regno);
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
u32 regno, u32 mem_size);
/* this lives here instead of in bpf.h because it needs to dereference tgt_prog */
static inline u64 bpf_trampoline_compute_key(const struct bpf_prog *tgt_prog,
......
......@@ -5291,15 +5291,16 @@ int btf_check_type_match(struct bpf_verifier_log *log, const struct bpf_prog *pr
* Only PTR_TO_CTX and SCALAR_VALUE states are recognized.
*/
int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
struct bpf_reg_state *reg)
struct bpf_reg_state *regs)
{
struct bpf_verifier_log *log = &env->log;
struct bpf_prog *prog = env->prog;
struct btf *btf = prog->aux->btf;
const struct btf_param *args;
const struct btf_type *t;
u32 i, nargs, btf_id;
const struct btf_type *t, *ref_t;
u32 i, nargs, btf_id, type_size;
const char *tname;
bool is_global;
if (!prog->aux->func_info)
return -EINVAL;
......@@ -5333,38 +5334,57 @@ int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
bpf_log(log, "Function %s has %d > 5 args\n", tname, nargs);
goto out;
}
is_global = prog->aux->func_info_aux[subprog].linkage == BTF_FUNC_GLOBAL;
/* check that BTF function arguments match actual types that the
* verifier sees.
*/
for (i = 0; i < nargs; i++) {
struct bpf_reg_state *reg = &regs[i + 1];
t = btf_type_by_id(btf, args[i].type);
while (btf_type_is_modifier(t))
t = btf_type_by_id(btf, t->type);
if (btf_type_is_int(t) || btf_type_is_enum(t)) {
if (reg[i + 1].type == SCALAR_VALUE)
if (reg->type == SCALAR_VALUE)
continue;
bpf_log(log, "R%d is not a scalar\n", i + 1);
goto out;
}
if (btf_type_is_ptr(t)) {
if (reg[i + 1].type == SCALAR_VALUE) {
bpf_log(log, "R%d is not a pointer\n", i + 1);
goto out;
}
/* If function expects ctx type in BTF check that caller
* is passing PTR_TO_CTX.
*/
if (btf_get_prog_ctx_type(log, btf, t, prog->type, i)) {
if (reg[i + 1].type != PTR_TO_CTX) {
if (reg->type != PTR_TO_CTX) {
bpf_log(log,
"arg#%d expected pointer to ctx, but got %s\n",
i, btf_kind_str[BTF_INFO_KIND(t->info)]);
goto out;
}
if (check_ctx_reg(env, &reg[i + 1], i + 1))
if (check_ctx_reg(env, reg, i + 1))
goto out;
continue;
}
if (!is_global)
goto out;
t = btf_type_skip_modifiers(btf, t->type, NULL);
ref_t = btf_resolve_size(btf, t, &type_size);
if (IS_ERR(ref_t)) {
bpf_log(log,
"arg#%d reference type('%s %s') size cannot be determined: %ld\n",
i, btf_type_str(t), btf_name_by_offset(btf, t->name_off),
PTR_ERR(ref_t));
goto out;
}
if (check_mem_reg(env, reg, i + 1, type_size))
goto out;
continue;
}
bpf_log(log, "Unrecognized arg#%d type %s\n",
i, btf_kind_str[BTF_INFO_KIND(t->info)]);
......@@ -5388,14 +5408,14 @@ int btf_check_func_arg_match(struct bpf_verifier_env *env, int subprog,
* (either PTR_TO_CTX or SCALAR_VALUE).
*/
int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
struct bpf_reg_state *reg)
struct bpf_reg_state *regs)
{
struct bpf_verifier_log *log = &env->log;
struct bpf_prog *prog = env->prog;
enum bpf_prog_type prog_type = prog->type;
struct btf *btf = prog->aux->btf;
const struct btf_param *args;
const struct btf_type *t;
const struct btf_type *t, *ref_t;
u32 i, nargs, btf_id;
const char *tname;
......@@ -5459,16 +5479,35 @@ int btf_prepare_func_args(struct bpf_verifier_env *env, int subprog,
* Only PTR_TO_CTX and SCALAR are supported atm.
*/
for (i = 0; i < nargs; i++) {
struct bpf_reg_state *reg = &regs[i + 1];
t = btf_type_by_id(btf, args[i].type);
while (btf_type_is_modifier(t))
t = btf_type_by_id(btf, t->type);
if (btf_type_is_int(t) || btf_type_is_enum(t)) {
reg[i + 1].type = SCALAR_VALUE;
reg->type = SCALAR_VALUE;
continue;
}
if (btf_type_is_ptr(t) &&
btf_get_prog_ctx_type(log, btf, t, prog_type, i)) {
reg[i + 1].type = PTR_TO_CTX;
if (btf_type_is_ptr(t)) {
if (btf_get_prog_ctx_type(log, btf, t, prog_type, i)) {
reg->type = PTR_TO_CTX;
continue;
}
t = btf_type_skip_modifiers(btf, t->type, NULL);
ref_t = btf_resolve_size(btf, t, &reg->mem_size);
if (IS_ERR(ref_t)) {
bpf_log(log,
"arg#%d reference type('%s %s') size cannot be determined: %ld\n",
i, btf_type_str(t), btf_name_by_offset(btf, t->name_off),
PTR_ERR(ref_t));
return -EINVAL;
}
reg->type = PTR_TO_MEM_OR_NULL;
reg->id = ++env->id_gen;
continue;
}
bpf_log(log, "Arg#%d type %s in %s() is not supported yet.\n",
......
......@@ -1079,6 +1079,51 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env,
__mark_reg_known_zero(regs + regno);
}
static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
{
switch (reg->type) {
case PTR_TO_MAP_VALUE_OR_NULL: {
const struct bpf_map *map = reg->map_ptr;
if (map->inner_map_meta) {
reg->type = CONST_PTR_TO_MAP;
reg->map_ptr = map->inner_map_meta;
} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
reg->type = PTR_TO_XDP_SOCK;
} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
map->map_type == BPF_MAP_TYPE_SOCKHASH) {
reg->type = PTR_TO_SOCKET;
} else {
reg->type = PTR_TO_MAP_VALUE;
}
break;
}
case PTR_TO_SOCKET_OR_NULL:
reg->type = PTR_TO_SOCKET;
break;
case PTR_TO_SOCK_COMMON_OR_NULL:
reg->type = PTR_TO_SOCK_COMMON;
break;
case PTR_TO_TCP_SOCK_OR_NULL:
reg->type = PTR_TO_TCP_SOCK;
break;
case PTR_TO_BTF_ID_OR_NULL:
reg->type = PTR_TO_BTF_ID;
break;
case PTR_TO_MEM_OR_NULL:
reg->type = PTR_TO_MEM;
break;
case PTR_TO_RDONLY_BUF_OR_NULL:
reg->type = PTR_TO_RDONLY_BUF;
break;
case PTR_TO_RDWR_BUF_OR_NULL:
reg->type = PTR_TO_RDWR_BUF;
break;
default:
WARN_ON("unknown nullable register type");
}
}
static bool reg_is_pkt_pointer(const struct bpf_reg_state *reg)
{
return type_is_pkt_pointer(reg->type);
......@@ -4227,6 +4272,29 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno,
}
}
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
u32 regno, u32 mem_size)
{
if (register_is_null(reg))
return 0;
if (reg_type_may_be_null(reg->type)) {
/* Assuming that the register contains a value check if the memory
* access is safe. Temporarily save and restore the register's state as
* the conversion shouldn't be visible to a caller.
*/
const struct bpf_reg_state saved_reg = *reg;
int rv;
mark_ptr_not_null_reg(reg);
rv = check_helper_mem_access(env, regno, mem_size, true, NULL);
*reg = saved_reg;
return rv;
}
return check_helper_mem_access(env, regno, mem_size, true, NULL);
}
/* Implementation details:
* bpf_map_lookup returns PTR_TO_MAP_VALUE_OR_NULL
* Two bpf_map_lookups (even with the same key) will have different reg->id.
......@@ -7737,43 +7805,19 @@ static void mark_ptr_or_null_reg(struct bpf_func_state *state,
}
if (is_null) {
reg->type = SCALAR_VALUE;
} else if (reg->type == PTR_TO_MAP_VALUE_OR_NULL) {
const struct bpf_map *map = reg->map_ptr;
if (map->inner_map_meta) {
reg->type = CONST_PTR_TO_MAP;
reg->map_ptr = map->inner_map_meta;
} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
reg->type = PTR_TO_XDP_SOCK;
} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
map->map_type == BPF_MAP_TYPE_SOCKHASH) {
reg->type = PTR_TO_SOCKET;
} else {
reg->type = PTR_TO_MAP_VALUE;
}
} else if (reg->type == PTR_TO_SOCKET_OR_NULL) {
reg->type = PTR_TO_SOCKET;
} else if (reg->type == PTR_TO_SOCK_COMMON_OR_NULL) {
reg->type = PTR_TO_SOCK_COMMON;
} else if (reg->type == PTR_TO_TCP_SOCK_OR_NULL) {
reg->type = PTR_TO_TCP_SOCK;
} else if (reg->type == PTR_TO_BTF_ID_OR_NULL) {
reg->type = PTR_TO_BTF_ID;
} else if (reg->type == PTR_TO_MEM_OR_NULL) {
reg->type = PTR_TO_MEM;
} else if (reg->type == PTR_TO_RDONLY_BUF_OR_NULL) {
reg->type = PTR_TO_RDONLY_BUF;
} else if (reg->type == PTR_TO_RDWR_BUF_OR_NULL) {
reg->type = PTR_TO_RDWR_BUF;
}
if (is_null) {
/* We don't need id and ref_obj_id from this point
* onwards anymore, thus we should better reset it,
* so that state pruning has chances to take effect.
*/
reg->id = 0;
reg->ref_obj_id = 0;
} else if (!reg_may_point_to_spin_lock(reg)) {
return;
}
mark_ptr_not_null_reg(reg);
if (!reg_may_point_to_spin_lock(reg)) {
/* For not-NULL ptr, reg->ref_obj_id will be reset
* in release_reg_references().
*
......@@ -11939,6 +11983,13 @@ static int do_check_common(struct bpf_verifier_env *env, int subprog)
mark_reg_known_zero(env, regs, i);
else if (regs[i].type == SCALAR_VALUE)
mark_reg_unknown(env, regs, i);
else if (regs[i].type == PTR_TO_MEM_OR_NULL) {
const u32 mem_size = regs[i].mem_size;
mark_reg_known_zero(env, regs, i);
regs[i].mem_size = mem_size;
regs[i].id = ++env->id_gen;
}
}
} else {
/* 1st arg to a function */
......
// SPDX-License-Identifier: GPL-2.0
#include "test_progs.h"
#include "network_helpers.h"
static __u32 duration;
static void test_global_func_args0(struct bpf_object *obj)
{
int err, i, map_fd, actual_value;
const char *map_name = "values";
map_fd = bpf_find_map(__func__, obj, map_name);
if (CHECK(map_fd < 0, "bpf_find_map", "cannot find BPF map %s: %s\n",
map_name, strerror(errno)))
return;
struct {
const char *descr;
int expected_value;
} tests[] = {
{"passing NULL pointer", 0},
{"returning value", 1},
{"reading local variable", 100 },
{"writing local variable", 101 },
{"reading global variable", 42 },
{"writing global variable", 43 },
{"writing to pointer-to-pointer", 1 },
};
for (i = 0; i < ARRAY_SIZE(tests); ++i) {
const int expected_value = tests[i].expected_value;
err = bpf_map_lookup_elem(map_fd, &i, &actual_value);
CHECK(err || actual_value != expected_value, tests[i].descr,
"err %d result %d expected %d\n", err, actual_value, expected_value);
}
}
void test_global_func_args(void)
{
const char *file = "./test_global_func_args.o";
__u32 retval;
struct bpf_object *obj;
int err, prog_fd;
err = bpf_prog_load(file, BPF_PROG_TYPE_CGROUP_SKB, &obj, &prog_fd);
if (CHECK(err, "load program", "error %d loading %s\n", err, file))
return;
err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
NULL, NULL, &retval, &duration);
CHECK(err || retval, "pass global func args run",
"err %d errno %d retval %d duration %d\n",
err, errno, retval, duration);
test_global_func_args0(obj);
bpf_object__close(obj);
}
......@@ -61,6 +61,14 @@ void test_test_global_funcs(void)
{ "test_global_func6.o" , "modified ctx ptr R2" },
{ "test_global_func7.o" , "foo() doesn't return scalar" },
{ "test_global_func8.o" },
{ "test_global_func9.o" },
{ "test_global_func10.o", "invalid indirect read from stack" },
{ "test_global_func11.o", "Caller passes invalid args into func#1" },
{ "test_global_func12.o", "invalid mem access 'mem_or_null'" },
{ "test_global_func13.o", "Caller passes invalid args into func#1" },
{ "test_global_func14.o", "reference type('FWD S') size cannot be determined" },
{ "test_global_func15.o", "At program exit the register R0 has value" },
{ "test_global_func16.o", "invalid indirect read from stack" },
};
libbpf_print_fn_t old_print_fn = NULL;
int err, i, duration = 0;
......
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct Small {
int x;
};
struct Big {
int x;
int y;
};
__noinline int foo(const struct Big *big)
{
if (big == 0)
return 0;
return bpf_get_prandom_u32() < big->y;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
const struct Small small = {.x = skb->len };
return foo((struct Big *)&small) ? 1 : 0;
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct S {
int x;
};
__noinline int foo(const struct S *s)
{
return s ? bpf_get_prandom_u32() < s->x : 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
return foo(skb);
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct S {
int x;
};
__noinline int foo(const struct S *s)
{
return bpf_get_prandom_u32() < s->x;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
const struct S s = {.x = skb->len };
return foo(&s);
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct S {
int x;
};
__noinline int foo(const struct S *s)
{
if (s)
return bpf_get_prandom_u32() < s->x;
return 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
const struct S *s = (const struct S *)(0xbedabeda);
return foo(s);
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct S;
__noinline int foo(const struct S *s)
{
if (s)
return bpf_get_prandom_u32() < *(const int *) s;
return 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
return foo(NULL);
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
__noinline int foo(unsigned int *v)
{
if (v)
*v = bpf_get_prandom_u32();
return 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
unsigned int v = 1;
foo(&v);
return v;
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
__noinline int foo(int (*arr)[10])
{
if (arr)
return (*arr)[9];
return 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
int array[10];
const int rv = foo(&array);
return rv ? 1 : 0;
}
// SPDX-License-Identifier: GPL-2.0-only
#include <stddef.h>
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct S {
int x;
};
struct C {
int x;
int y;
};
struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(max_entries, 1);
__type(key, __u32);
__type(value, struct S);
} map SEC(".maps");
enum E {
E_ITEM
};
static int global_data_x = 100;
static int volatile global_data_y = 500;
__noinline int foo(const struct S *s)
{
if (s)
return bpf_get_prandom_u32() < s->x;
return 0;
}
__noinline int bar(int *x)
{
if (x)
*x &= bpf_get_prandom_u32();
return 0;
}
__noinline int baz(volatile int *x)
{
if (x)
*x &= bpf_get_prandom_u32();
return 0;
}
__noinline int qux(enum E *e)
{
if (e)
return *e;
return 0;
}
__noinline int quux(int (*arr)[10])
{
if (arr)
return (*arr)[9];
return 0;
}
__noinline int quuz(int **p)
{
if (p)
*p = NULL;
return 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
int result = 0;
{
const struct S s = {.x = skb->len };
result |= foo(&s);
}
{
const __u32 key = 1;
const struct S *s = bpf_map_lookup_elem(&map, &key);
result |= foo(s);
}
{
const struct C c = {.x = skb->len, .y = skb->family };
result |= foo((const struct S *)&c);
}
{
result |= foo(NULL);
}
{
bar(&result);
bar(&global_data_x);
}
{
result |= baz(&global_data_y);
}
{
enum E e = E_ITEM;
result |= qux(&e);
}
{
int array[10] = {0};
result |= quux(&array);
}
{
int *p;
result |= quuz(&p);
}
return result ? 1 : 0;
}
// SPDX-License-Identifier: GPL-2.0
#include <linux/bpf.h>
#include <bpf/bpf_helpers.h>
struct S {
int v;
};
static volatile struct S global_variable;
struct {
__uint(type, BPF_MAP_TYPE_ARRAY);
__uint(max_entries, 7);
__type(key, __u32);
__type(value, int);
} values SEC(".maps");
static void save_value(__u32 index, int value)
{
bpf_map_update_elem(&values, &index, &value, 0);
}
__noinline int foo(__u32 index, struct S *s)
{
if (s) {
save_value(index, s->v);
return ++s->v;
}
save_value(index, 0);
return 1;
}
__noinline int bar(__u32 index, volatile struct S *s)
{
if (s) {
save_value(index, s->v);
return ++s->v;
}
save_value(index, 0);
return 1;
}
__noinline int baz(struct S **s)
{
if (s)
*s = 0;
return 0;
}
SEC("cgroup_skb/ingress")
int test_cls(struct __sk_buff *skb)
{
__u32 index = 0;
{
const int v = foo(index++, 0);
save_value(index++, v);
}
{
struct S s = { .v = 100 };
foo(index++, &s);
save_value(index++, s.v);
}
{
global_variable.v = 42;
bar(index++, &global_variable);
save_value(index++, global_variable.v);
}
{
struct S v, *p = &v;
baz(&p);
save_value(index++, !p);
}
return 0;
}
char _license[] SEC("license") = "GPL";
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment