Skip to content

Commit

Permalink
bpf: Treat first argument as return value for bpf_throw
Browse files Browse the repository at this point in the history
In case of the default exception callback, change the behavior of
bpf_throw, where the passed cookie value is no longer ignored, but
is instead the return value of the default exception callback. As
such, we need to place restrictions on the value being passed into
bpf_throw in such a case, only allowing those permitted by the
check_return_code function.

Thus, bpf_throw can now control the return value of the program from
each call site without having the user install a custom exception
callback just to override the return value when an exception is thrown.

We also modify the hidden subprog instructions to now move BPF_REG_1 to
BPF_REG_0, so as to set the return value before exit in the default
callback.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
Link: https://lore.kernel.org/r/20230912233214.1518551-9-memxor@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
  • Loading branch information
kkdwivedi authored and Alexei Starovoitov committed Sep 16, 2023
1 parent b62bf8a commit a923819
Showing 1 changed file with 24 additions and 13 deletions.
37 changes: 24 additions & 13 deletions kernel/bpf/verifier.c
Original file line number Diff line number Diff line change
Expand Up @@ -11485,6 +11485,8 @@ static int fetch_kfunc_meta(struct bpf_verifier_env *env,
return 0;
}

static int check_return_code(struct bpf_verifier_env *env, int regno);

static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
int *insn_idx_p)
{
Expand Down Expand Up @@ -11613,6 +11615,15 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
return -ENOTSUPP;
}
env->seen_exception = true;

/* In the case of the default callback, the cookie value passed
* to bpf_throw becomes the return value of the program.
*/
if (!env->exception_callback_subprog) {
err = check_return_code(env, BPF_REG_1);
if (err < 0)
return err;
}
}

for (i = 0; i < CALLER_SAVED_REGS; i++)
Expand Down Expand Up @@ -14709,7 +14720,7 @@ static int check_ld_abs(struct bpf_verifier_env *env, struct bpf_insn *insn)
return 0;
}

static int check_return_code(struct bpf_verifier_env *env)
static int check_return_code(struct bpf_verifier_env *env, int regno)
{
struct tnum enforce_attach_type_range = tnum_unknown;
const struct bpf_prog *prog = env->prog;
Expand Down Expand Up @@ -14743,22 +14754,22 @@ static int check_return_code(struct bpf_verifier_env *env)
* of bpf_exit, which means that program wrote
* something into it earlier
*/
err = check_reg_arg(env, BPF_REG_0, SRC_OP);
err = check_reg_arg(env, regno, SRC_OP);
if (err)
return err;

if (is_pointer_value(env, BPF_REG_0)) {
verbose(env, "R0 leaks addr as return value\n");
if (is_pointer_value(env, regno)) {
verbose(env, "R%d leaks addr as return value\n", regno);
return -EACCES;
}

reg = cur_regs(env) + BPF_REG_0;
reg = cur_regs(env) + regno;

if (frame->in_async_callback_fn) {
/* enforce return zero from async callbacks like timer */
if (reg->type != SCALAR_VALUE) {
verbose(env, "In async callback the register R0 is not a known value (%s)\n",
reg_type_str(env, reg->type));
verbose(env, "In async callback the register R%d is not a known value (%s)\n",
regno, reg_type_str(env, reg->type));
return -EINVAL;
}

Expand All @@ -14771,8 +14782,8 @@ static int check_return_code(struct bpf_verifier_env *env)

if (is_subprog && !frame->in_exception_callback_fn) {
if (reg->type != SCALAR_VALUE) {
verbose(env, "At subprogram exit the register R0 is not a scalar value (%s)\n",
reg_type_str(env, reg->type));
verbose(env, "At subprogram exit the register R%d is not a scalar value (%s)\n",
regno, reg_type_str(env, reg->type));
return -EINVAL;
}
return 0;
Expand Down Expand Up @@ -14854,8 +14865,8 @@ static int check_return_code(struct bpf_verifier_env *env)
}

if (reg->type != SCALAR_VALUE) {
verbose(env, "At program exit the register R0 is not a known value (%s)\n",
reg_type_str(env, reg->type));
verbose(env, "At program exit the register R%d is not a known value (%s)\n",
regno, reg_type_str(env, reg->type));
return -EINVAL;
}

Expand Down Expand Up @@ -17053,7 +17064,7 @@ static int do_check(struct bpf_verifier_env *env)
continue;
}

err = check_return_code(env);
err = check_return_code(env, BPF_REG_0);
if (err)
return err;
process_bpf_exit:
Expand Down Expand Up @@ -18722,7 +18733,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
if (env->seen_exception && !env->exception_callback_subprog) {
struct bpf_insn patch[] = {
env->prog->insnsi[insn_cnt - 1],
BPF_MOV64_IMM(BPF_REG_0, 0),
BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
BPF_EXIT_INSN(),
};

Expand Down

0 comments on commit a923819

Please sign in to comment.