Commit f50b49a0 authored by KP Singh's avatar KP Singh Committed by Alexei Starovoitov

bpf: btf: Fix arg verification in btf_ctx_access()

The bounds checking for the arguments accessed in the BPF program breaks
when the expected_attach_type is not BPF_TRACE_FEXIT, BPF_LSM_MAC or
BPF_MODIFY_RETURN resulting in no check being done for the default case
(the programs which do not receive the return value of the attached
function in its arguments) when the index of the argument being accessed
is equal to the number of arguments (nr_args).

This was a result of a misplaced "else if" block  introduced by the
Commit 6ba43b76 ("bpf: Attachment verification for
BPF_MODIFY_RETURN")

Fixes: 6ba43b76 ("bpf: Attachment verification for BPF_MODIFY_RETURN")
Reported-by: default avatarJann Horn <jannh@google.com>
Signed-off-by: default avatarKP Singh <kpsingh@google.com>
Signed-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/20200330144246.338-1-kpsingh@chromium.org
parent 0fc31b10
...@@ -3709,9 +3709,16 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, ...@@ -3709,9 +3709,16 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
nr_args--; nr_args--;
} }
if (arg > nr_args) {
bpf_log(log, "func '%s' doesn't have %d-th argument\n",
tname, arg + 1);
return false;
}
if (arg == nr_args) { if (arg == nr_args) {
if (prog->expected_attach_type == BPF_TRACE_FEXIT || switch (prog->expected_attach_type) {
prog->expected_attach_type == BPF_LSM_MAC) { case BPF_LSM_MAC:
case BPF_TRACE_FEXIT:
/* When LSM programs are attached to void LSM hooks /* When LSM programs are attached to void LSM hooks
* they use FEXIT trampolines and when attached to * they use FEXIT trampolines and when attached to
* int LSM hooks, they use MODIFY_RETURN trampolines. * int LSM hooks, they use MODIFY_RETURN trampolines.
...@@ -3728,7 +3735,8 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, ...@@ -3728,7 +3735,8 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
if (!t) if (!t)
return true; return true;
t = btf_type_by_id(btf, t->type); t = btf_type_by_id(btf, t->type);
} else if (prog->expected_attach_type == BPF_MODIFY_RETURN) { break;
case BPF_MODIFY_RETURN:
/* For now the BPF_MODIFY_RETURN can only be attached to /* For now the BPF_MODIFY_RETURN can only be attached to
* functions that return an int. * functions that return an int.
*/ */
...@@ -3742,17 +3750,19 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type, ...@@ -3742,17 +3750,19 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
btf_kind_str[BTF_INFO_KIND(t->info)]); btf_kind_str[BTF_INFO_KIND(t->info)]);
return false; return false;
} }
break;
default:
bpf_log(log, "func '%s' doesn't have %d-th argument\n",
tname, arg + 1);
return false;
} }
} else if (arg >= nr_args) {
bpf_log(log, "func '%s' doesn't have %d-th argument\n",
tname, arg + 1);
return false;
} else { } else {
if (!t) if (!t)
/* Default prog with 5 args */ /* Default prog with 5 args */
return true; return true;
t = btf_type_by_id(btf, args[arg].type); t = btf_type_by_id(btf, args[arg].type);
} }
/* skip modifiers */ /* skip modifiers */
while (btf_type_is_modifier(t)) while (btf_type_is_modifier(t))
t = btf_type_by_id(btf, t->type); t = btf_type_by_id(btf, t->type);
......
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