Merge branch 'Dynptr refactorings'

Kumar Kartikeya Dwivedi says:

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

This is part 1 of https://lore.kernel.org/bpf/20221018135920.726360-1-memxor@gmail.com.
This thread also gives some background on why the refactor is being done:
https://lore.kernel.org/bpf/CAEf4Bzb4beTHgVo+G+jehSj8oCeAjRbRcm6MRe=Gr+cajRBwEw@mail.gmail.com

As requested in patch 6 by Alexei, it only includes patches which
refactors the code, on top of which further fixes will be made in part
2. The refactor itself fixes another issue as a side effect. No
functional change is intended (except a few modified log messages).

Changelog:
----------
v1 -> v2
v1: https://lore.kernel.org/bpf/20221115000130.1967465-1-memxor@gmail.com

 * Address feedback from Joanne and David, add acks

Fixes v1 -> v1
Fixes v1: https://lore.kernel.org/bpf/20221018135920.726360-1-memxor@gmail.com

 * Collect acks from Joanne and David
 * Fix misc nits pointed out by Joanne, David
 * Split move of reg->off alignment check for dynptr into separate
   change (Alexei)
====================

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
Alexei Starovoitov 2022-12-08 18:25:32 -08:00
commit 26d6506a50
13 changed files with 370 additions and 203 deletions

View file

@ -775,7 +775,7 @@ enum bpf_reg_type {
PTR_TO_MEM, /* reg points to valid memory region */
PTR_TO_BUF, /* reg points to a read/write buffer */
PTR_TO_FUNC, /* reg points to a bpf program function */
PTR_TO_DYNPTR, /* reg points to a dynptr */
CONST_PTR_TO_DYNPTR, /* reg points to a const struct bpf_dynptr */
__BPF_REG_TYPE_MAX,
/* Extended reg_types. */
@ -2828,7 +2828,7 @@ void bpf_dynptr_init(struct bpf_dynptr_kern *ptr, void *data,
enum bpf_dynptr_type type, u32 offset, u32 size);
void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr);
int bpf_dynptr_check_size(u32 size);
u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr);
u32 bpf_dynptr_get_size(const struct bpf_dynptr_kern *ptr);
#ifdef CONFIG_BPF_LSM
void bpf_cgroup_atype_get(u32 attach_btf_id, int cgroup_atype);

View file

@ -615,11 +615,9 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
enum bpf_arg_type arg_type);
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
u32 regno, u32 mem_size);
bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,
struct bpf_reg_state *reg);
bool is_dynptr_type_expected(struct bpf_verifier_env *env,
struct bpf_reg_state *reg,
enum bpf_arg_type arg_type);
struct bpf_call_arg_meta;
int process_dynptr_func(struct bpf_verifier_env *env, int regno,
enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta);
/* 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,

View file

@ -5293,7 +5293,7 @@ union bpf_attr {
* Return
* Nothing. Always succeeds.
*
* long bpf_dynptr_read(void *dst, u32 len, struct bpf_dynptr *src, u32 offset, u64 flags)
* long bpf_dynptr_read(void *dst, u32 len, const struct bpf_dynptr *src, u32 offset, u64 flags)
* Description
* Read *len* bytes from *src* into *dst*, starting from *offset*
* into *src*.
@ -5303,7 +5303,7 @@ union bpf_attr {
* of *src*'s data, -EINVAL if *src* is an invalid dynptr or if
* *flags* is not 0.
*
* long bpf_dynptr_write(struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags)
* long bpf_dynptr_write(const struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags)
* Description
* Write *len* bytes from *src* into *dst*, starting from *offset*
* into *dst*.
@ -5313,7 +5313,7 @@ union bpf_attr {
* of *dst*'s data, -EINVAL if *dst* is an invalid dynptr or if *dst*
* is a read-only dynptr or if *flags* is not 0.
*
* void *bpf_dynptr_data(struct bpf_dynptr *ptr, u32 offset, u32 len)
* void *bpf_dynptr_data(const struct bpf_dynptr *ptr, u32 offset, u32 len)
* Description
* Get a pointer to the underlying dynptr data.
*
@ -5414,7 +5414,7 @@ union bpf_attr {
* Drain samples from the specified user ring buffer, and invoke
* the provided callback for each such sample:
*
* long (\*callback_fn)(struct bpf_dynptr \*dynptr, void \*ctx);
* long (\*callback_fn)(const struct bpf_dynptr \*dynptr, void \*ctx);
*
* If **callback_fn** returns 0, the helper will continue to try
* and drain the next sample, up to a maximum of

View file

@ -1404,7 +1404,7 @@ static const struct bpf_func_proto bpf_kptr_xchg_proto = {
#define DYNPTR_SIZE_MASK 0xFFFFFF
#define DYNPTR_RDONLY_BIT BIT(31)
static bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr)
static bool bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr)
{
return ptr->size & DYNPTR_RDONLY_BIT;
}
@ -1414,7 +1414,7 @@ static void bpf_dynptr_set_type(struct bpf_dynptr_kern *ptr, enum bpf_dynptr_typ
ptr->size |= type << DYNPTR_TYPE_SHIFT;
}
u32 bpf_dynptr_get_size(struct bpf_dynptr_kern *ptr)
u32 bpf_dynptr_get_size(const struct bpf_dynptr_kern *ptr)
{
return ptr->size & DYNPTR_SIZE_MASK;
}
@ -1438,7 +1438,7 @@ void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr)
memset(ptr, 0, sizeof(*ptr));
}
static int bpf_dynptr_check_off_len(struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
static int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
{
u32 size = bpf_dynptr_get_size(ptr);
@ -1483,7 +1483,7 @@ static const struct bpf_func_proto bpf_dynptr_from_mem_proto = {
.arg4_type = ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL | MEM_UNINIT,
};
BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, struct bpf_dynptr_kern *, src,
BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, const struct bpf_dynptr_kern *, src,
u32, offset, u64, flags)
{
int err;
@ -1495,7 +1495,11 @@ BPF_CALL_5(bpf_dynptr_read, void *, dst, u32, len, struct bpf_dynptr_kern *, src
if (err)
return err;
memcpy(dst, src->data + src->offset + offset, len);
/* Source and destination may possibly overlap, hence use memmove to
* copy the data. E.g. bpf_dynptr_from_mem may create two dynptr
* pointing to overlapping PTR_TO_MAP_VALUE regions.
*/
memmove(dst, src->data + src->offset + offset, len);
return 0;
}
@ -1506,12 +1510,12 @@ static const struct bpf_func_proto bpf_dynptr_read_proto = {
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_UNINIT_MEM,
.arg2_type = ARG_CONST_SIZE_OR_ZERO,
.arg3_type = ARG_PTR_TO_DYNPTR,
.arg3_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY,
.arg4_type = ARG_ANYTHING,
.arg5_type = ARG_ANYTHING,
};
BPF_CALL_5(bpf_dynptr_write, struct bpf_dynptr_kern *, dst, u32, offset, void *, src,
BPF_CALL_5(bpf_dynptr_write, const struct bpf_dynptr_kern *, dst, u32, offset, void *, src,
u32, len, u64, flags)
{
int err;
@ -1523,7 +1527,11 @@ BPF_CALL_5(bpf_dynptr_write, struct bpf_dynptr_kern *, dst, u32, offset, void *,
if (err)
return err;
memcpy(dst->data + dst->offset + offset, src, len);
/* Source and destination may possibly overlap, hence use memmove to
* copy the data. E.g. bpf_dynptr_from_mem may create two dynptr
* pointing to overlapping PTR_TO_MAP_VALUE regions.
*/
memmove(dst->data + dst->offset + offset, src, len);
return 0;
}
@ -1532,14 +1540,14 @@ static const struct bpf_func_proto bpf_dynptr_write_proto = {
.func = bpf_dynptr_write,
.gpl_only = false,
.ret_type = RET_INTEGER,
.arg1_type = ARG_PTR_TO_DYNPTR,
.arg1_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY,
.arg2_type = ARG_ANYTHING,
.arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY,
.arg4_type = ARG_CONST_SIZE_OR_ZERO,
.arg5_type = ARG_ANYTHING,
};
BPF_CALL_3(bpf_dynptr_data, struct bpf_dynptr_kern *, ptr, u32, offset, u32, len)
BPF_CALL_3(bpf_dynptr_data, const struct bpf_dynptr_kern *, ptr, u32, offset, u32, len)
{
int err;
@ -1560,7 +1568,7 @@ static const struct bpf_func_proto bpf_dynptr_data_proto = {
.func = bpf_dynptr_data,
.gpl_only = false,
.ret_type = RET_PTR_TO_DYNPTR_MEM_OR_NULL,
.arg1_type = ARG_PTR_TO_DYNPTR,
.arg1_type = ARG_PTR_TO_DYNPTR | MEM_RDONLY,
.arg2_type = ARG_ANYTHING,
.arg3_type = ARG_CONST_ALLOC_SIZE_OR_ZERO,
};

View file

@ -592,7 +592,7 @@ static const char *reg_type_str(struct bpf_verifier_env *env,
[PTR_TO_BUF] = "buf",
[PTR_TO_FUNC] = "func",
[PTR_TO_MAP_KEY] = "map_key",
[PTR_TO_DYNPTR] = "dynptr_ptr",
[CONST_PTR_TO_DYNPTR] = "dynptr_ptr",
};
if (type & PTR_MAYBE_NULL) {
@ -725,6 +725,28 @@ static bool dynptr_type_refcounted(enum bpf_dynptr_type type)
return type == BPF_DYNPTR_TYPE_RINGBUF;
}
static void __mark_dynptr_reg(struct bpf_reg_state *reg,
enum bpf_dynptr_type type,
bool first_slot);
static void __mark_reg_not_init(const struct bpf_verifier_env *env,
struct bpf_reg_state *reg);
static void mark_dynptr_stack_regs(struct bpf_reg_state *sreg1,
struct bpf_reg_state *sreg2,
enum bpf_dynptr_type type)
{
__mark_dynptr_reg(sreg1, type, true);
__mark_dynptr_reg(sreg2, type, false);
}
static void mark_dynptr_cb_reg(struct bpf_reg_state *reg,
enum bpf_dynptr_type type)
{
__mark_dynptr_reg(reg, type, true);
}
static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
enum bpf_arg_type arg_type, int insn_idx)
{
@ -746,9 +768,8 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
if (type == BPF_DYNPTR_TYPE_INVALID)
return -EINVAL;
state->stack[spi].spilled_ptr.dynptr.first_slot = true;
state->stack[spi].spilled_ptr.dynptr.type = type;
state->stack[spi - 1].spilled_ptr.dynptr.type = type;
mark_dynptr_stack_regs(&state->stack[spi].spilled_ptr,
&state->stack[spi - 1].spilled_ptr, type);
if (dynptr_type_refcounted(type)) {
/* The id is used to track proper releasing */
@ -756,8 +777,8 @@ static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
if (id < 0)
return id;
state->stack[spi].spilled_ptr.id = id;
state->stack[spi - 1].spilled_ptr.id = id;
state->stack[spi].spilled_ptr.ref_obj_id = id;
state->stack[spi - 1].spilled_ptr.ref_obj_id = id;
}
return 0;
@ -779,25 +800,23 @@ static int unmark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_re
}
/* Invalidate any slices associated with this dynptr */
if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type)) {
release_reference(env, state->stack[spi].spilled_ptr.id);
state->stack[spi].spilled_ptr.id = 0;
state->stack[spi - 1].spilled_ptr.id = 0;
}
state->stack[spi].spilled_ptr.dynptr.first_slot = false;
state->stack[spi].spilled_ptr.dynptr.type = 0;
state->stack[spi - 1].spilled_ptr.dynptr.type = 0;
if (dynptr_type_refcounted(state->stack[spi].spilled_ptr.dynptr.type))
WARN_ON_ONCE(release_reference(env, state->stack[spi].spilled_ptr.ref_obj_id));
__mark_reg_not_init(env, &state->stack[spi].spilled_ptr);
__mark_reg_not_init(env, &state->stack[spi - 1].spilled_ptr);
return 0;
}
static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
{
struct bpf_func_state *state = func(env, reg);
int spi = get_spi(reg->off);
int i;
int spi, i;
if (reg->type == CONST_PTR_TO_DYNPTR)
return false;
spi = get_spi(reg->off);
if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS))
return true;
@ -810,13 +829,17 @@ static bool is_dynptr_reg_valid_uninit(struct bpf_verifier_env *env, struct bpf_
return true;
}
bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,
struct bpf_reg_state *reg)
static bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
{
struct bpf_func_state *state = func(env, reg);
int spi = get_spi(reg->off);
int spi;
int i;
/* This already represents first slot of initialized bpf_dynptr */
if (reg->type == CONST_PTR_TO_DYNPTR)
return true;
spi = get_spi(reg->off);
if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) ||
!state->stack[spi].spilled_ptr.dynptr.first_slot)
return false;
@ -830,21 +853,24 @@ bool is_dynptr_reg_valid_init(struct bpf_verifier_env *env,
return true;
}
bool is_dynptr_type_expected(struct bpf_verifier_env *env,
struct bpf_reg_state *reg,
enum bpf_arg_type arg_type)
static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
enum bpf_arg_type arg_type)
{
struct bpf_func_state *state = func(env, reg);
enum bpf_dynptr_type dynptr_type;
int spi = get_spi(reg->off);
int spi;
/* ARG_PTR_TO_DYNPTR takes any type of dynptr */
if (arg_type == ARG_PTR_TO_DYNPTR)
return true;
dynptr_type = arg_to_dynptr_type(arg_type);
return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type;
if (reg->type == CONST_PTR_TO_DYNPTR) {
return reg->dynptr.type == dynptr_type;
} else {
spi = get_spi(reg->off);
return state->stack[spi].spilled_ptr.dynptr.type == dynptr_type;
}
}
/* The reg state of a pointer or a bounded scalar was saved when
@ -1356,9 +1382,6 @@ static const int caller_saved[CALLER_SAVED_REGS] = {
BPF_REG_0, BPF_REG_1, BPF_REG_2, BPF_REG_3, BPF_REG_4, BPF_REG_5
};
static void __mark_reg_not_init(const struct bpf_verifier_env *env,
struct bpf_reg_state *reg);
/* This helper doesn't clear reg->id */
static void ___mark_reg_known(struct bpf_reg_state *reg, u64 imm)
{
@ -1421,6 +1444,19 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env,
__mark_reg_known_zero(regs + regno);
}
static void __mark_dynptr_reg(struct bpf_reg_state *reg, enum bpf_dynptr_type type,
bool first_slot)
{
/* reg->type has no meaning for STACK_DYNPTR, but when we set reg for
* callback arguments, it does need to be CONST_PTR_TO_DYNPTR, so simply
* set it unconditionally as it is ignored for STACK_DYNPTR anyway.
*/
__mark_reg_known_zero(reg);
reg->type = CONST_PTR_TO_DYNPTR;
reg->dynptr.type = type;
reg->dynptr.first_slot = first_slot;
}
static void mark_ptr_not_null_reg(struct bpf_reg_state *reg)
{
if (base_type(reg->type) == PTR_TO_MAP_VALUE) {
@ -5859,6 +5895,119 @@ static int process_kptr_func(struct bpf_verifier_env *env, int regno,
return 0;
}
/* There are two register types representing a bpf_dynptr, one is PTR_TO_STACK
* which points to a stack slot, and the other is CONST_PTR_TO_DYNPTR.
*
* In both cases we deal with the first 8 bytes, but need to mark the next 8
* bytes as STACK_DYNPTR in case of PTR_TO_STACK. In case of
* CONST_PTR_TO_DYNPTR, we are guaranteed to get the beginning of the object.
*
* Mutability of bpf_dynptr is at two levels, one is at the level of struct
* bpf_dynptr itself, i.e. whether the helper is receiving a pointer to struct
* bpf_dynptr or pointer to const struct bpf_dynptr. In the former case, it can
* mutate the view of the dynptr and also possibly destroy it. In the latter
* case, it cannot mutate the bpf_dynptr itself but it can still mutate the
* memory that dynptr points to.
*
* The verifier will keep track both levels of mutation (bpf_dynptr's in
* reg->type and the memory's in reg->dynptr.type), but there is no support for
* readonly dynptr view yet, hence only the first case is tracked and checked.
*
* This is consistent with how C applies the const modifier to a struct object,
* where the pointer itself inside bpf_dynptr becomes const but not what it
* points to.
*
* Helpers which do not mutate the bpf_dynptr set MEM_RDONLY in their argument
* type, and declare it as 'const struct bpf_dynptr *' in their prototype.
*/
int process_dynptr_func(struct bpf_verifier_env *env, int regno,
enum bpf_arg_type arg_type, struct bpf_call_arg_meta *meta)
{
struct bpf_reg_state *regs = cur_regs(env), *reg = &regs[regno];
/* MEM_UNINIT and MEM_RDONLY are exclusive, when applied to an
* ARG_PTR_TO_DYNPTR (or ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_*):
*/
if ((arg_type & (MEM_UNINIT | MEM_RDONLY)) == (MEM_UNINIT | MEM_RDONLY)) {
verbose(env, "verifier internal error: misconfigured dynptr helper type flags\n");
return -EFAULT;
}
/* CONST_PTR_TO_DYNPTR already has fixed and var_off as 0 due to
* check_func_arg_reg_off's logic. We only need to check offset
* alignment for PTR_TO_STACK.
*/
if (reg->type == PTR_TO_STACK && (reg->off % BPF_REG_SIZE)) {
verbose(env, "cannot pass in dynptr at an offset=%d\n", reg->off);
return -EINVAL;
}
/* MEM_UNINIT - Points to memory that is an appropriate candidate for
* constructing a mutable bpf_dynptr object.
*
* Currently, this is only possible with PTR_TO_STACK
* pointing to a region of at least 16 bytes which doesn't
* contain an existing bpf_dynptr.
*
* MEM_RDONLY - Points to a initialized bpf_dynptr that will not be
* mutated or destroyed. However, the memory it points to
* may be mutated.
*
* None - Points to a initialized dynptr that can be mutated and
* destroyed, including mutation of the memory it points
* to.
*/
if (arg_type & MEM_UNINIT) {
if (!is_dynptr_reg_valid_uninit(env, reg)) {
verbose(env, "Dynptr has to be an uninitialized dynptr\n");
return -EINVAL;
}
/* We only support one dynptr being uninitialized at the moment,
* which is sufficient for the helper functions we have right now.
*/
if (meta->uninit_dynptr_regno) {
verbose(env, "verifier internal error: multiple uninitialized dynptr args\n");
return -EFAULT;
}
meta->uninit_dynptr_regno = regno;
} else /* MEM_RDONLY and None case from above */ {
/* For the reg->type == PTR_TO_STACK case, bpf_dynptr is never const */
if (reg->type == CONST_PTR_TO_DYNPTR && !(arg_type & MEM_RDONLY)) {
verbose(env, "cannot pass pointer to const bpf_dynptr, the helper mutates it\n");
return -EINVAL;
}
if (!is_dynptr_reg_valid_init(env, reg)) {
verbose(env,
"Expected an initialized dynptr as arg #%d\n",
regno);
return -EINVAL;
}
/* Fold modifiers (in this case, MEM_RDONLY) when checking expected type */
if (!is_dynptr_type_expected(env, reg, arg_type & ~MEM_RDONLY)) {
const char *err_extra = "";
switch (arg_type & DYNPTR_TYPE_FLAG_MASK) {
case DYNPTR_TYPE_LOCAL:
err_extra = "local";
break;
case DYNPTR_TYPE_RINGBUF:
err_extra = "ringbuf";
break;
default:
err_extra = "<unknown>";
break;
}
verbose(env,
"Expected a dynptr of type %s as arg #%d\n",
err_extra, regno);
return -EINVAL;
}
}
return 0;
}
static bool arg_type_is_mem_size(enum bpf_arg_type type)
{
return type == ARG_CONST_SIZE ||
@ -5999,7 +6148,7 @@ static const struct bpf_reg_types kptr_types = { .types = { PTR_TO_MAP_VALUE } }
static const struct bpf_reg_types dynptr_types = {
.types = {
PTR_TO_STACK,
PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL,
CONST_PTR_TO_DYNPTR,
}
};
@ -6128,17 +6277,38 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
const struct bpf_reg_state *reg, int regno,
enum bpf_arg_type arg_type)
{
enum bpf_reg_type type = reg->type;
bool fixed_off_ok = false;
u32 type = reg->type;
switch ((u32)type) {
/* Pointer types where reg offset is explicitly allowed: */
case PTR_TO_STACK:
if (arg_type_is_dynptr(arg_type) && reg->off % BPF_REG_SIZE) {
verbose(env, "cannot pass in dynptr at an offset\n");
/* When referenced register is passed to release function, its fixed
* offset must be 0.
*
* We will check arg_type_is_release reg has ref_obj_id when storing
* meta->release_regno.
*/
if (arg_type_is_release(arg_type)) {
/* ARG_PTR_TO_DYNPTR with OBJ_RELEASE is a bit special, as it
* may not directly point to the object being released, but to
* dynptr pointing to such object, which might be at some offset
* on the stack. In that case, we simply to fallback to the
* default handling.
*/
if (arg_type_is_dynptr(arg_type) && type == PTR_TO_STACK)
return 0;
/* Doing check_ptr_off_reg check for the offset will catch this
* because fixed_off_ok is false, but checking here allows us
* to give the user a better error message.
*/
if (reg->off) {
verbose(env, "R%d must have zero offset when passed to release func or trusted arg to kfunc\n",
regno);
return -EINVAL;
}
fallthrough;
return __check_ptr_off_reg(env, reg, regno, false);
}
switch (type) {
/* Pointer types where both fixed and variable offset is explicitly allowed: */
case PTR_TO_STACK:
case PTR_TO_PACKET:
case PTR_TO_PACKET_META:
case PTR_TO_MAP_KEY:
@ -6149,12 +6319,7 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
case PTR_TO_BUF:
case PTR_TO_BUF | MEM_RDONLY:
case SCALAR_VALUE:
/* Some of the argument types nevertheless require a
* zero register offset.
*/
if (base_type(arg_type) != ARG_PTR_TO_RINGBUF_MEM)
return 0;
break;
return 0;
/* All the rest must be rejected, except PTR_TO_BTF_ID which allows
* fixed offset.
*/
@ -6164,32 +6329,28 @@ int check_func_arg_reg_off(struct bpf_verifier_env *env,
case PTR_TO_BTF_ID | MEM_RCU:
case PTR_TO_BTF_ID | MEM_ALLOC | PTR_TRUSTED:
/* When referenced PTR_TO_BTF_ID is passed to release function,
* it's fixed offset must be 0. In the other cases, fixed offset
* can be non-zero.
* its fixed offset must be 0. In the other cases, fixed offset
* can be non-zero. This was already checked above. So pass
* fixed_off_ok as true to allow fixed offset for all other
* cases. var_off always must be 0 for PTR_TO_BTF_ID, hence we
* still need to do checks instead of returning.
*/
if (arg_type_is_release(arg_type) && reg->off) {
verbose(env, "R%d must have zero offset when passed to release func\n",
regno);
return -EINVAL;
}
/* For arg is release pointer, fixed_off_ok must be false, but
* we already checked and rejected reg->off != 0 above, so set
* to true to allow fixed offset for all other cases.
*/
fixed_off_ok = true;
break;
return __check_ptr_off_reg(env, reg, regno, true);
default:
break;
return __check_ptr_off_reg(env, reg, regno, false);
}
return __check_ptr_off_reg(env, reg, regno, fixed_off_ok);
}
static u32 stack_slot_get_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
static u32 dynptr_ref_obj_id(struct bpf_verifier_env *env, struct bpf_reg_state *reg)
{
struct bpf_func_state *state = func(env, reg);
int spi = get_spi(reg->off);
int spi;
return state->stack[spi].spilled_ptr.id;
if (reg->type == CONST_PTR_TO_DYNPTR)
return reg->ref_obj_id;
spi = get_spi(reg->off);
return state->stack[spi].spilled_ptr.ref_obj_id;
}
static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
@ -6254,11 +6415,22 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
if (arg_type_is_release(arg_type)) {
if (arg_type_is_dynptr(arg_type)) {
struct bpf_func_state *state = func(env, reg);
int spi = get_spi(reg->off);
int spi;
if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) ||
!state->stack[spi].spilled_ptr.id) {
verbose(env, "arg %d is an unacquired reference\n", regno);
/* Only dynptr created on stack can be released, thus
* the get_spi and stack state checks for spilled_ptr
* should only be done before process_dynptr_func for
* PTR_TO_STACK.
*/
if (reg->type == PTR_TO_STACK) {
spi = get_spi(reg->off);
if (!is_spi_bounds_valid(state, spi, BPF_DYNPTR_NR_SLOTS) ||
!state->stack[spi].spilled_ptr.ref_obj_id) {
verbose(env, "arg %d is an unacquired reference\n", regno);
return -EINVAL;
}
} else {
verbose(env, "cannot release unowned const bpf_dynptr\n");
return -EINVAL;
}
} else if (!reg->ref_obj_id && !register_is_null(reg)) {
@ -6355,19 +6527,22 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
break;
case ARG_PTR_TO_SPIN_LOCK:
if (meta->func_id == BPF_FUNC_spin_lock) {
if (process_spin_lock(env, regno, true))
return -EACCES;
err = process_spin_lock(env, regno, true);
if (err)
return err;
} else if (meta->func_id == BPF_FUNC_spin_unlock) {
if (process_spin_lock(env, regno, false))
return -EACCES;
err = process_spin_lock(env, regno, false);
if (err)
return err;
} else {
verbose(env, "verifier internal error\n");
return -EFAULT;
}
break;
case ARG_PTR_TO_TIMER:
if (process_timer_func(env, regno, meta))
return -EACCES;
err = process_timer_func(env, regno, meta);
if (err)
return err;
break;
case ARG_PTR_TO_FUNC:
meta->subprogno = reg->subprogno;
@ -6390,52 +6565,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
err = check_mem_size_reg(env, reg, regno, true, meta);
break;
case ARG_PTR_TO_DYNPTR:
/* We only need to check for initialized / uninitialized helper
* dynptr args if the dynptr is not PTR_TO_DYNPTR, as the
* assumption is that if it is, that a helper function
* initialized the dynptr on behalf of the BPF program.
*/
if (base_type(reg->type) == PTR_TO_DYNPTR)
break;
if (arg_type & MEM_UNINIT) {
if (!is_dynptr_reg_valid_uninit(env, reg)) {
verbose(env, "Dynptr has to be an uninitialized dynptr\n");
return -EINVAL;
}
/* We only support one dynptr being uninitialized at the moment,
* which is sufficient for the helper functions we have right now.
*/
if (meta->uninit_dynptr_regno) {
verbose(env, "verifier internal error: multiple uninitialized dynptr args\n");
return -EFAULT;
}
meta->uninit_dynptr_regno = regno;
} else if (!is_dynptr_reg_valid_init(env, reg)) {
verbose(env,
"Expected an initialized dynptr as arg #%d\n",
arg + 1);
return -EINVAL;
} else if (!is_dynptr_type_expected(env, reg, arg_type)) {
const char *err_extra = "";
switch (arg_type & DYNPTR_TYPE_FLAG_MASK) {
case DYNPTR_TYPE_LOCAL:
err_extra = "local";
break;
case DYNPTR_TYPE_RINGBUF:
err_extra = "ringbuf";
break;
default:
err_extra = "<unknown>";
break;
}
verbose(env,
"Expected a dynptr of type %s as arg #%d\n",
err_extra, arg + 1);
return -EINVAL;
}
err = process_dynptr_func(env, regno, arg_type, meta);
if (err)
return err;
break;
case ARG_CONST_ALLOC_SIZE_OR_ZERO:
if (!tnum_is_const(reg->var_off)) {
@ -6502,8 +6634,9 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
break;
}
case ARG_PTR_TO_KPTR:
if (process_kptr_func(env, regno, meta))
return -EACCES;
err = process_kptr_func(env, regno, meta);
if (err)
return err;
break;
}
@ -7271,11 +7404,10 @@ static int set_user_ringbuf_callback_state(struct bpf_verifier_env *env,
{
/* bpf_user_ringbuf_drain(struct bpf_map *map, void *callback_fn, void
* callback_ctx, u64 flags);
* callback_fn(struct bpf_dynptr_t* dynptr, void *callback_ctx);
* callback_fn(const struct bpf_dynptr_t* dynptr, void *callback_ctx);
*/
__mark_reg_not_init(env, &callee->regs[BPF_REG_0]);
callee->regs[BPF_REG_1].type = PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL;
__mark_reg_known_zero(&callee->regs[BPF_REG_1]);
mark_dynptr_cb_reg(&callee->regs[BPF_REG_1], BPF_DYNPTR_TYPE_LOCAL);
callee->regs[BPF_REG_2] = caller->regs[BPF_REG_3];
/* unused */
@ -7669,7 +7801,15 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
regs = cur_regs(env);
/* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot
* be reinitialized by any dynptr helper. Hence, mark_stack_slots_dynptr
* is safe to do directly.
*/
if (meta.uninit_dynptr_regno) {
if (regs[meta.uninit_dynptr_regno].type == CONST_PTR_TO_DYNPTR) {
verbose(env, "verifier internal error: CONST_PTR_TO_DYNPTR cannot be initialized\n");
return -EFAULT;
}
/* we write BPF_DW bits (8 bytes) at a time */
for (i = 0; i < BPF_DYNPTR_SIZE; i += 8) {
err = check_mem_access(env, insn_idx, meta.uninit_dynptr_regno,
@ -7687,15 +7827,24 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
if (meta.release_regno) {
err = -EINVAL;
if (arg_type_is_dynptr(fn->arg_type[meta.release_regno - BPF_REG_1]))
err = unmark_stack_slots_dynptr(env, &regs[meta.release_regno]);
else if (meta.ref_obj_id)
err = release_reference(env, meta.ref_obj_id);
/* meta.ref_obj_id can only be 0 if register that is meant to be
* released is NULL, which must be > R0.
/* This can only be set for PTR_TO_STACK, as CONST_PTR_TO_DYNPTR cannot
* be released by any dynptr helper. Hence, unmark_stack_slots_dynptr
* is safe to do directly.
*/
else if (register_is_null(&regs[meta.release_regno]))
if (arg_type_is_dynptr(fn->arg_type[meta.release_regno - BPF_REG_1])) {
if (regs[meta.release_regno].type == CONST_PTR_TO_DYNPTR) {
verbose(env, "verifier internal error: CONST_PTR_TO_DYNPTR cannot be released\n");
return -EFAULT;
}
err = unmark_stack_slots_dynptr(env, &regs[meta.release_regno]);
} else if (meta.ref_obj_id) {
err = release_reference(env, meta.ref_obj_id);
} else if (register_is_null(&regs[meta.release_regno])) {
/* meta.ref_obj_id can only be 0 if register that is meant to be
* released is NULL, which must be > R0.
*/
err = 0;
}
if (err) {
verbose(env, "func %s#%d reference has not been acquired before\n",
func_id_name(func_id), func_id);
@ -7769,11 +7918,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
return -EFAULT;
}
if (base_type(reg->type) != PTR_TO_DYNPTR)
/* Find the id of the dynptr we're
* tracking the reference of
*/
meta.ref_obj_id = stack_slot_get_id(env, reg);
meta.ref_obj_id = dynptr_ref_obj_id(env, reg);
break;
}
}
@ -8829,22 +8974,15 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
return ret;
break;
case KF_ARG_PTR_TO_DYNPTR:
if (reg->type != PTR_TO_STACK) {
verbose(env, "arg#%d expected pointer to stack\n", i);
if (reg->type != PTR_TO_STACK &&
reg->type != CONST_PTR_TO_DYNPTR) {
verbose(env, "arg#%d expected pointer to stack or dynptr_ptr\n", i);
return -EINVAL;
}
if (!is_dynptr_reg_valid_init(env, reg)) {
verbose(env, "arg#%d pointer type %s %s must be valid and initialized\n",
i, btf_type_str(ref_t), ref_tname);
return -EINVAL;
}
if (!is_dynptr_type_expected(env, reg, ARG_PTR_TO_DYNPTR | DYNPTR_TYPE_LOCAL)) {
verbose(env, "arg#%d pointer type %s %s points to unsupported dynamic pointer type\n",
i, btf_type_str(ref_t), ref_tname);
return -EINVAL;
}
ret = process_dynptr_func(env, regno, ARG_PTR_TO_DYNPTR | MEM_RDONLY, NULL);
if (ret < 0)
return ret;
break;
case KF_ARG_PTR_TO_LIST_HEAD:
if (reg->type != PTR_TO_MAP_VALUE &&

View file

@ -752,6 +752,7 @@ class PrinterHelpers(Printer):
'struct bpf_timer',
'struct mptcp_sock',
'struct bpf_dynptr',
'const struct bpf_dynptr',
'struct iphdr',
'struct ipv6hdr',
}

View file

@ -5293,7 +5293,7 @@ union bpf_attr {
* Return
* Nothing. Always succeeds.
*
* long bpf_dynptr_read(void *dst, u32 len, struct bpf_dynptr *src, u32 offset, u64 flags)
* long bpf_dynptr_read(void *dst, u32 len, const struct bpf_dynptr *src, u32 offset, u64 flags)
* Description
* Read *len* bytes from *src* into *dst*, starting from *offset*
* into *src*.
@ -5303,7 +5303,7 @@ union bpf_attr {
* of *src*'s data, -EINVAL if *src* is an invalid dynptr or if
* *flags* is not 0.
*
* long bpf_dynptr_write(struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags)
* long bpf_dynptr_write(const struct bpf_dynptr *dst, u32 offset, void *src, u32 len, u64 flags)
* Description
* Write *len* bytes from *src* into *dst*, starting from *offset*
* into *dst*.
@ -5313,7 +5313,7 @@ union bpf_attr {
* of *dst*'s data, -EINVAL if *dst* is an invalid dynptr or if *dst*
* is a read-only dynptr or if *flags* is not 0.
*
* void *bpf_dynptr_data(struct bpf_dynptr *ptr, u32 offset, u32 len)
* void *bpf_dynptr_data(const struct bpf_dynptr *ptr, u32 offset, u32 len)
* Description
* Get a pointer to the underlying dynptr data.
*
@ -5414,7 +5414,7 @@ union bpf_attr {
* Drain samples from the specified user ring buffer, and invoke
* the provided callback for each such sample:
*
* long (\*callback_fn)(struct bpf_dynptr \*dynptr, void \*ctx);
* long (\*callback_fn)(const struct bpf_dynptr \*dynptr, void \*ctx);
*
* If **callback_fn** returns 0, the helper will continue to try
* and drain the next sample, up to a maximum of

View file

@ -18,11 +18,8 @@ static struct {
const char *expected_verifier_err_msg;
int expected_runtime_err;
} kfunc_dynptr_tests[] = {
{"dynptr_type_not_supp",
"arg#0 pointer type STRUCT bpf_dynptr_kern points to unsupported dynamic pointer type", 0},
{"not_valid_dynptr",
"arg#0 pointer type STRUCT bpf_dynptr_kern must be valid and initialized", 0},
{"not_ptr_to_stack", "arg#0 expected pointer to stack", 0},
{"not_valid_dynptr", "Expected an initialized dynptr as arg #1", 0},
{"not_ptr_to_stack", "arg#0 expected pointer to stack or dynptr_ptr", 0},
{"dynptr_data_null", NULL, -EBADMSG},
};

View file

@ -673,9 +673,11 @@ static struct {
{"user_ringbuf_callback_write_forbidden", "invalid mem access 'dynptr_ptr'"},
{"user_ringbuf_callback_null_context_write", "invalid mem access 'scalar'"},
{"user_ringbuf_callback_null_context_read", "invalid mem access 'scalar'"},
{"user_ringbuf_callback_discard_dynptr", "arg 1 is an unacquired reference"},
{"user_ringbuf_callback_submit_dynptr", "arg 1 is an unacquired reference"},
{"user_ringbuf_callback_discard_dynptr", "cannot release unowned const bpf_dynptr"},
{"user_ringbuf_callback_submit_dynptr", "cannot release unowned const bpf_dynptr"},
{"user_ringbuf_callback_invalid_return", "At callback return the register R0 has value"},
{"user_ringbuf_callback_reinit_dynptr_mem", "Dynptr has to be an uninitialized dynptr"},
{"user_ringbuf_callback_reinit_dynptr_ringbuf", "Dynptr has to be an uninitialized dynptr"},
};
#define SUCCESS_TEST(_func) { _func, #_func }

View file

@ -32,18 +32,6 @@ int err, pid;
char _license[] SEC("license") = "GPL";
SEC("?lsm.s/bpf")
int BPF_PROG(dynptr_type_not_supp, int cmd, union bpf_attr *attr,
unsigned int size)
{
char write_data[64] = "hello there, world!!";
struct bpf_dynptr ptr;
bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(write_data), 0, &ptr);
return bpf_verify_pkcs7_signature(&ptr, &ptr, NULL);
}
SEC("?lsm.s/bpf")
int BPF_PROG(not_valid_dynptr, int cmd, union bpf_attr *attr, unsigned int size)
{

View file

@ -18,6 +18,13 @@ struct {
__uint(type, BPF_MAP_TYPE_USER_RINGBUF);
} user_ringbuf SEC(".maps");
struct {
__uint(type, BPF_MAP_TYPE_RINGBUF);
__uint(max_entries, 2);
} ringbuf SEC(".maps");
static int map_value;
static long
bad_access1(struct bpf_dynptr *dynptr, void *context)
{
@ -32,7 +39,7 @@ bad_access1(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to read before the pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_bad_access1(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, bad_access1, NULL, 0);
@ -54,7 +61,7 @@ bad_access2(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to read past the end of the pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_bad_access2(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, bad_access2, NULL, 0);
@ -73,7 +80,7 @@ write_forbidden(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to write to that pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_write_forbidden(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, write_forbidden, NULL, 0);
@ -92,7 +99,7 @@ null_context_write(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to write to that pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_null_context_write(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, null_context_write, NULL, 0);
@ -113,7 +120,7 @@ null_context_read(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to write to that pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_null_context_read(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, null_context_read, NULL, 0);
@ -132,7 +139,7 @@ try_discard_dynptr(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to read past the end of the pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_discard_dynptr(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, try_discard_dynptr, NULL, 0);
@ -151,7 +158,7 @@ try_submit_dynptr(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to read past the end of the pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_submit_dynptr(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, try_submit_dynptr, NULL, 0);
@ -168,10 +175,38 @@ invalid_drain_callback_return(struct bpf_dynptr *dynptr, void *context)
/* A callback that accesses a dynptr in a bpf_user_ringbuf_drain callback should
* not be able to write to that pointer.
*/
SEC("?raw_tp/sys_nanosleep")
SEC("?raw_tp/")
int user_ringbuf_callback_invalid_return(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, invalid_drain_callback_return, NULL, 0);
return 0;
}
static long
try_reinit_dynptr_mem(struct bpf_dynptr *dynptr, void *context)
{
bpf_dynptr_from_mem(&map_value, 4, 0, dynptr);
return 0;
}
static long
try_reinit_dynptr_ringbuf(struct bpf_dynptr *dynptr, void *context)
{
bpf_ringbuf_reserve_dynptr(&ringbuf, 8, 0, dynptr);
return 0;
}
SEC("?raw_tp/")
int user_ringbuf_callback_reinit_dynptr_mem(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, try_reinit_dynptr_mem, NULL, 0);
return 0;
}
SEC("?raw_tp/")
int user_ringbuf_callback_reinit_dynptr_ringbuf(void *ctx)
{
bpf_user_ringbuf_drain(&user_ringbuf, try_reinit_dynptr_ringbuf, NULL, 0);
return 0;
}

View file

@ -76,7 +76,7 @@
},
.prog_type = BPF_PROG_TYPE_SCHED_CLS,
.result = REJECT,
.errstr = "arg#0 expected pointer to ctx, but got PTR",
.errstr = "R1 must have zero offset when passed to release func or trusted arg to kfunc",
.fixup_kfunc_btf_id = {
{ "bpf_kfunc_call_test_pass_ctx", 2 },
},

View file

@ -28,7 +28,7 @@
},
.fixup_map_ringbuf = { 1 },
.result = REJECT,
.errstr = "dereference of modified ringbuf_mem ptr R1",
.errstr = "R1 must have zero offset when passed to release func",
},
{
"ringbuf: invalid reservation offset 2",