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: 6ba43b761c41 ("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
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 8d7ee40..bfea3f9 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10428,22 +10428,13 @@
}
#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) ||
!strncmp(SECURITY_PREFIX, prog->aux->attach_func_name,
sizeof(SECURITY_PREFIX) - 1))
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;
}
@@ -10654,11 +10645,18 @@
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;
prog->aux->trampoline = tr;
-
- if (prog->expected_attach_type == BPF_MODIFY_RETURN)
- ret = check_attach_modify_return(env);
out:
mutex_unlock(&tr->mutex);
if (ret)
