bpf: Fix use-after-free in fmod_ret check

Fix the following issue:
[  436.749342] BUG: KASAN: use-after-free in bpf_trampoline_put+0x39/0x2a0
[  436.749995] Write of size 4 at addr ffff8881ef38b8a0 by task kworker/3:5/2243
[  436.750712]
[  436.752677] Workqueue: events bpf_prog_free_deferred
[  436.753183] Call Trace:
[  436.756483]  bpf_trampoline_put+0x39/0x2a0
[  436.756904]  bpf_prog_free_deferred+0x16d/0x3d0
[  436.757377]  process_one_work+0x94a/0x15b0
[  436.761969]
[  436.762130] Allocated by task 2529:
[  436.763323]  bpf_trampoline_lookup+0x136/0x540
[  436.763776]  bpf_check+0x2872/0xa0a8
[  436.764144]  bpf_prog_load+0xb6f/0x1350
[  436.764539]  __do_sys_bpf+0x16d7/0x3720
[  436.765825]
[  436.765988] Freed by task 2529:
[  436.767084]  kfree+0xc6/0x280
[  436.767397]  bpf_trampoline_put+0x1fd/0x2a0
[  436.767826]  bpf_check+0x6832/0xa0a8
[  436.768197]  bpf_prog_load+0xb6f/0x1350
[  436.768594]  __do_sys_bpf+0x16d7/0x3720

prog->aux->trampoline = tr should be set only when prog is valid.
Otherwise prog freeing will try to put trampoline via prog->aux->trampoline,
but it may not point to a valid trampoline.

Fixes: 6ba43b761c ("bpf: Attachment verification for BPF_MODIFY_RETURN")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: KP Singh <kpsingh@google.com>
Link: https://lore.kernel.org/bpf/20200529043839.15824-2-alexei.starovoitov@gmail.com
This commit is contained in:
Alexei Starovoitov 2020-05-28 21:38:36 -07:00 committed by Daniel Borkmann
parent d195b1d1d1
commit 18644cec71
1 changed files with 11 additions and 13 deletions

View File

@ -10428,22 +10428,13 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
} }
#define SECURITY_PREFIX "security_" #define SECURITY_PREFIX "security_"
static int check_attach_modify_return(struct bpf_verifier_env *env) static int check_attach_modify_return(struct bpf_prog *prog, unsigned long addr)
{ {
struct bpf_prog *prog = env->prog;
unsigned long addr = (unsigned long) prog->aux->trampoline->func.addr;
/* This is expected to be cleaned up in the future with the KRSI effort
* introducing the LSM_HOOK macro for cleaning up lsm_hooks.h.
*/
if (within_error_injection_list(addr) || if (within_error_injection_list(addr) ||
!strncmp(SECURITY_PREFIX, prog->aux->attach_func_name, !strncmp(SECURITY_PREFIX, prog->aux->attach_func_name,
sizeof(SECURITY_PREFIX) - 1)) sizeof(SECURITY_PREFIX) - 1))
return 0; return 0;
verbose(env, "fmod_ret attach_btf_id %u (%s) is not modifiable\n",
prog->aux->attach_btf_id, prog->aux->attach_func_name);
return -EINVAL; return -EINVAL;
} }
@ -10654,11 +10645,18 @@ static int check_attach_btf_id(struct bpf_verifier_env *env)
goto out; goto out;
} }
} }
if (prog->expected_attach_type == BPF_MODIFY_RETURN) {
ret = check_attach_modify_return(prog, addr);
if (ret)
verbose(env, "%s() is not modifiable\n",
prog->aux->attach_func_name);
}
if (ret)
goto out;
tr->func.addr = (void *)addr; tr->func.addr = (void *)addr;
prog->aux->trampoline = tr; prog->aux->trampoline = tr;
if (prog->expected_attach_type == BPF_MODIFY_RETURN)
ret = check_attach_modify_return(env);
out: out:
mutex_unlock(&tr->mutex); mutex_unlock(&tr->mutex);
if (ret) if (ret)