Skip to content

Commit 6788ab2

Browse files
committed
bpf: Generally fix helper register offset check
Right now the assertion on check_ptr_off_reg() is only enforced for register types PTR_TO_CTX (and open coded also for PTR_TO_BTF_ID), however, this is insufficient since many other PTR_TO_* register types such as PTR_TO_FUNC do not handle/expect register offsets when passed to helper functions. Given this can slip-through easily when adding new types, make this an explicit allow-list and reject all other current and future types by default if this is encountered. Also, extend check_ptr_off_reg() to handle PTR_TO_BTF_ID as well instead of duplicating it. For PTR_TO_BTF_ID, reg->off is used for BTF to match expected BTF ids if struct offset is used. This part still needs to be allowed, but the dynamic off from the tnum must be rejected. Fixes: 69c087b ("bpf: Add bpf_for_each_map_elem() helper") Fixes: eaa6bcb ("bpf: Introduce bpf_per_cpu_ptr()") Signed-off-by: Daniel Borkmann <[email protected]> Acked-by: John Fastabend <[email protected]> Acked-by: Alexei Starovoitov <[email protected]>
1 parent d400a6c commit 6788ab2

File tree

1 file changed

+28
-11
lines changed

1 file changed

+28
-11
lines changed

kernel/bpf/verifier.c

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3969,14 +3969,15 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env,
39693969
}
39703970
#endif
39713971

3972-
int check_ptr_off_reg(struct bpf_verifier_env *env,
3973-
const struct bpf_reg_state *reg, int regno)
3972+
static int __check_ptr_off_reg(struct bpf_verifier_env *env,
3973+
const struct bpf_reg_state *reg, int regno,
3974+
bool fixed_off_ok)
39743975
{
39753976
/* Access to this pointer-typed register or passing it to a helper
39763977
* is only allowed in its original, unmodified form.
39773978
*/
39783979

3979-
if (reg->off) {
3980+
if (!fixed_off_ok && reg->off) {
39803981
verbose(env, "dereference of modified %s ptr R%d off=%d disallowed\n",
39813982
reg_type_str(env, reg->type), regno, reg->off);
39823983
return -EACCES;
@@ -3994,6 +3995,12 @@ int check_ptr_off_reg(struct bpf_verifier_env *env,
39943995
return 0;
39953996
}
39963997

3998+
int check_ptr_off_reg(struct bpf_verifier_env *env,
3999+
const struct bpf_reg_state *reg, int regno)
4000+
{
4001+
return __check_ptr_off_reg(env, reg, regno, false);
4002+
}
4003+
39974004
static int __check_buffer_access(struct bpf_verifier_env *env,
39984005
const char *buf_info,
39994006
const struct bpf_reg_state *reg,
@@ -5245,12 +5252,6 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno,
52455252
kernel_type_name(btf_vmlinux, *arg_btf_id));
52465253
return -EACCES;
52475254
}
5248-
5249-
if (!tnum_is_const(reg->var_off) || reg->var_off.value) {
5250-
verbose(env, "R%d is a pointer to in-kernel struct with non-zero offset\n",
5251-
regno);
5252-
return -EACCES;
5253-
}
52545255
}
52555256

52565257
return 0;
@@ -5305,10 +5306,26 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
53055306
if (err)
53065307
return err;
53075308

5308-
if (type == PTR_TO_CTX) {
5309-
err = check_ptr_off_reg(env, reg, regno);
5309+
switch ((u32)type) {
5310+
case SCALAR_VALUE:
5311+
/* Pointer types where reg offset is explicitly allowed: */
5312+
case PTR_TO_PACKET:
5313+
case PTR_TO_PACKET_META:
5314+
case PTR_TO_MAP_KEY:
5315+
case PTR_TO_MAP_VALUE:
5316+
case PTR_TO_MEM:
5317+
case PTR_TO_MEM | MEM_RDONLY:
5318+
case PTR_TO_BUF:
5319+
case PTR_TO_BUF | MEM_RDONLY:
5320+
case PTR_TO_STACK:
5321+
break;
5322+
/* All the rest must be rejected: */
5323+
default:
5324+
err = __check_ptr_off_reg(env, reg, regno,
5325+
type == PTR_TO_BTF_ID);
53105326
if (err < 0)
53115327
return err;
5328+
break;
53125329
}
53135330

53145331
skip_type_check:

0 commit comments

Comments
 (0)