Commit be77354a authored by Kumar Kartikeya Dwivedi's avatar Kumar Kartikeya Dwivedi Committed by Alexei Starovoitov

bpf: Do write access check for kfunc and global func

When passing pointer to some map value to kfunc or global func, in
verifier we are passing meta as NULL to various functions, which uses
meta->raw_mode to check whether memory is being written to. Since some
kfunc or global funcs may also write to memory pointers they receive as
arguments, we must check for write access to memory. E.g. in some case
map may be read only and this will be missed by current checks.

However meta->raw_mode allows for uninitialized memory (e.g. on stack),
since there is not enough info available through BTF, we must perform
one call for read access (raw_mode = false), and one for write access
(raw_mode = true).

Fixes: e5069b9c ("bpf: Support pointers in global func args")
Fixes: d583691c ("bpf: Introduce mem, size argument pair support for kfunc")
Signed-off-by: default avatarKumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20220319080827.73251-2-memxor@gmail.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent ebaf24c5
...@@ -4919,8 +4919,7 @@ static int check_mem_size_reg(struct bpf_verifier_env *env, ...@@ -4919,8 +4919,7 @@ static int check_mem_size_reg(struct bpf_verifier_env *env,
* out. Only upper bounds can be learned because retval is an * out. Only upper bounds can be learned because retval is an
* int type and negative retvals are allowed. * int type and negative retvals are allowed.
*/ */
if (meta) meta->msize_max_value = reg->umax_value;
meta->msize_max_value = reg->umax_value;
/* The register is SCALAR_VALUE; the access check /* The register is SCALAR_VALUE; the access check
* happens using its boundaries. * happens using its boundaries.
...@@ -4963,24 +4962,33 @@ static int check_mem_size_reg(struct bpf_verifier_env *env, ...@@ -4963,24 +4962,33 @@ static int check_mem_size_reg(struct bpf_verifier_env *env,
int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, int check_mem_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
u32 regno, u32 mem_size) u32 regno, u32 mem_size)
{ {
bool may_be_null = type_may_be_null(reg->type);
struct bpf_reg_state saved_reg;
struct bpf_call_arg_meta meta;
int err;
if (register_is_null(reg)) if (register_is_null(reg))
return 0; return 0;
if (type_may_be_null(reg->type)) { memset(&meta, 0, sizeof(meta));
/* Assuming that the register contains a value check if the memory /* Assuming that the register contains a value check if the memory
* access is safe. Temporarily save and restore the register's state as * access is safe. Temporarily save and restore the register's state as
* the conversion shouldn't be visible to a caller. * the conversion shouldn't be visible to a caller.
*/ */
const struct bpf_reg_state saved_reg = *reg; if (may_be_null) {
int rv; saved_reg = *reg;
mark_ptr_not_null_reg(reg); 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); err = check_helper_mem_access(env, regno, mem_size, true, &meta);
/* Check access for BPF_WRITE */
meta.raw_mode = true;
err = err ?: check_helper_mem_access(env, regno, mem_size, true, &meta);
if (may_be_null)
*reg = saved_reg;
return err;
} }
int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg, int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
...@@ -4989,16 +4997,22 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state ...@@ -4989,16 +4997,22 @@ int check_kfunc_mem_size_reg(struct bpf_verifier_env *env, struct bpf_reg_state
struct bpf_reg_state *mem_reg = &cur_regs(env)[regno - 1]; struct bpf_reg_state *mem_reg = &cur_regs(env)[regno - 1];
bool may_be_null = type_may_be_null(mem_reg->type); bool may_be_null = type_may_be_null(mem_reg->type);
struct bpf_reg_state saved_reg; struct bpf_reg_state saved_reg;
struct bpf_call_arg_meta meta;
int err; int err;
WARN_ON_ONCE(regno < BPF_REG_2 || regno > BPF_REG_5); WARN_ON_ONCE(regno < BPF_REG_2 || regno > BPF_REG_5);
memset(&meta, 0, sizeof(meta));
if (may_be_null) { if (may_be_null) {
saved_reg = *mem_reg; saved_reg = *mem_reg;
mark_ptr_not_null_reg(mem_reg); mark_ptr_not_null_reg(mem_reg);
} }
err = check_mem_size_reg(env, reg, regno, true, NULL); err = check_mem_size_reg(env, reg, regno, true, &meta);
/* Check access for BPF_WRITE */
meta.raw_mode = true;
err = err ?: check_mem_size_reg(env, reg, regno, true, &meta);
if (may_be_null) if (may_be_null)
*mem_reg = saved_reg; *mem_reg = saved_reg;
......
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