btf_datasec_resolve contains a bug that causes the following BTF
to fail loading:
[1] DATASEC a size=2 vlen=2
type_id=4 offset=0 size=1
type_id=7 offset=1 size=1
[2] INT (anon) size=1 bits_offset=0 nr_bits=8 encoding=(none)
[3] PTR (anon) type_id=2
[4] VAR a type_id=3 linkage=0
[5] INT (anon) size=1 bits_offset=0 nr_bits=8 encoding=(none)
[6] TYPEDEF td type_id=5
[7] VAR b type_id=6 linkage=0
This error message is printed during btf_check_all_types:
[1] DATASEC a size=2 vlen=2
type_id=7 offset=1 size=1 Invalid type
By tracing btf_*_resolve we can pinpoint the problem:
btf_datasec_resolve(depth: 1, type_id: 1, mode: RESOLVE_TBD) = 0
btf_var_resolve(depth: 2, type_id: 4, mode: RESOLVE_TBD) = 0
btf_ptr_resolve(depth: 3, type_id: 3, mode: RESOLVE_PTR) = 0
btf_var_resolve(depth: 2, type_id: 4, mode: RESOLVE_PTR) = 0
btf_datasec_resolve(depth: 1, type_id: 1, mode: RESOLVE_PTR) = -22
The last invocation of btf_datasec_resolve should invoke btf_var_resolve
by means of env_stack_push, instead it returns EINVAL. The reason is that
env_stack_push is never executed for the second VAR.
if (!env_type_is_resolve_sink(env, var_type) &&
!env_type_is_resolved(env, var_type_id)) {
env_stack_set_next_member(env, i + 1);
return env_stack_push(env, var_type, var_type_id);
}
env_type_is_resolve_sink() changes its behaviour based on resolve_mode.
For RESOLVE_PTR, we can simplify the if condition to the following:
(btf_type_is_modifier() || btf_type_is_ptr) && !env_type_is_resolved()
Since we're dealing with a VAR the clause evaluates to false. This is
not sufficient to trigger the bug however. The log output and EINVAL
are only generated if btf_type_id_size() fails.
if (!btf_type_id_size(btf, &type_id, &type_size)) {
btf_verifier_log_vsi(env, v->t, vsi, "Invalid type");
return -EINVAL;
}
Most types are sized, so for example a VAR referring to an INT is not a
problem. The bug is only triggered if a VAR points at a modifier. Since
we skipped btf_var_resolve that modifier was also never resolved, which
means that btf_resolved_type_id returns 0 aka VOID for the modifier.
This in turn causes btf_type_id_size to return NULL, triggering EINVAL.
To summarise, the following conditions are necessary:
- VAR pointing at PTR, STRUCT, UNION or ARRAY
- Followed by a VAR pointing at TYPEDEF, VOLATILE, CONST, RESTRICT or
TYPE_TAG
The fix is to reset resolve_mode to RESOLVE_TBD before attempting to
resolve a VAR from a DATASEC.
Fixes: 1dc92851849c ("bpf: kernel side support for BTF Var and DataSec")
Signed-off-by: Lorenz Bauer <[email protected]>
---
kernel/bpf/btf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index fa22ec79ac0e..91145298c238 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -4579,6 +4579,7 @@ static int btf_datasec_resolve(struct btf_verifier_env *env,
return -EINVAL;
}
+ env->resolve_mode = RESOLVE_TBD;
if (!env_type_is_resolve_sink(env, var_type) &&
!env_type_is_resolved(env, var_type_id)) {
env_stack_set_next_member(env, i + 1);
--
2.39.2
On 3/2/23 4:34 AM, Lorenz Bauer wrote:
> btf_datasec_resolve contains a bug that causes the following BTF
> to fail loading:
>
> [1] DATASEC a size=2 vlen=2
> type_id=4 offset=0 size=1
> type_id=7 offset=1 size=1
> [2] INT (anon) size=1 bits_offset=0 nr_bits=8 encoding=(none)
> [3] PTR (anon) type_id=2
> [4] VAR a type_id=3 linkage=0
> [5] INT (anon) size=1 bits_offset=0 nr_bits=8 encoding=(none)
> [6] TYPEDEF td type_id=5
> [7] VAR b type_id=6 linkage=0
>
> This error message is printed during btf_check_all_types:
>
> [1] DATASEC a size=2 vlen=2
> type_id=7 offset=1 size=1 Invalid type
>
> By tracing btf_*_resolve we can pinpoint the problem:
>
> btf_datasec_resolve(depth: 1, type_id: 1, mode: RESOLVE_TBD) = 0
> btf_var_resolve(depth: 2, type_id: 4, mode: RESOLVE_TBD) = 0
> btf_ptr_resolve(depth: 3, type_id: 3, mode: RESOLVE_PTR) = 0
> btf_var_resolve(depth: 2, type_id: 4, mode: RESOLVE_PTR) = 0
> btf_datasec_resolve(depth: 1, type_id: 1, mode: RESOLVE_PTR) = -22
>
> The last invocation of btf_datasec_resolve should invoke btf_var_resolve
> by means of env_stack_push, instead it returns EINVAL. The reason is that
> env_stack_push is never executed for the second VAR.
>
> if (!env_type_is_resolve_sink(env, var_type) &&
> !env_type_is_resolved(env, var_type_id)) {
> env_stack_set_next_member(env, i + 1);
> return env_stack_push(env, var_type, var_type_id);
> }
>
> env_type_is_resolve_sink() changes its behaviour based on resolve_mode.
> For RESOLVE_PTR, we can simplify the if condition to the following:
>
> (btf_type_is_modifier() || btf_type_is_ptr) && !env_type_is_resolved()
>
> Since we're dealing with a VAR the clause evaluates to false. This is
> not sufficient to trigger the bug however. The log output and EINVAL
> are only generated if btf_type_id_size() fails.
>
> if (!btf_type_id_size(btf, &type_id, &type_size)) {
> btf_verifier_log_vsi(env, v->t, vsi, "Invalid type");
> return -EINVAL;
> }
>
> Most types are sized, so for example a VAR referring to an INT is not a
> problem. The bug is only triggered if a VAR points at a modifier. Since
> we skipped btf_var_resolve that modifier was also never resolved, which
> means that btf_resolved_type_id returns 0 aka VOID for the modifier.
> This in turn causes btf_type_id_size to return NULL, triggering EINVAL.
>
> To summarise, the following conditions are necessary:
>
> - VAR pointing at PTR, STRUCT, UNION or ARRAY
> - Followed by a VAR pointing at TYPEDEF, VOLATILE, CONST, RESTRICT or
> TYPE_TAG
>
> The fix is to reset resolve_mode to RESOLVE_TBD before attempting to
> resolve a VAR from a DATASEC.
>
> Fixes: 1dc92851849c ("bpf: kernel side support for BTF Var and DataSec")
> Signed-off-by: Lorenz Bauer <[email protected]>
> ---
> kernel/bpf/btf.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index fa22ec79ac0e..91145298c238 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -4579,6 +4579,7 @@ static int btf_datasec_resolve(struct btf_verifier_env *env,
> return -EINVAL;
> }
>
> + env->resolve_mode = RESOLVE_TBD;
lgtm. Could it be moved out of the for loop?
Please add the test case described in the commit message to the prog_tests/btf.c.
> if (!env_type_is_resolve_sink(env, var_type) &&
> !env_type_is_resolved(env, var_type_id)) {
> env_stack_set_next_member(env, i + 1);
On Fri, Mar 3, 2023 at 12:16 AM Martin KaFai Lau <[email protected]> wrote:
>
> lgtm. Could it be moved out of the for loop?
Yeah, that is possible I think, since we can only trigger the problem if
return env_stack_push()
Is executed. I'll send a v2.
> Please add the test case described in the commit message to the prog_tests/btf.c.
Good point, I'll send this as a separate commit. Backporting patches
doesn't work well in my experience.
Thanks for your review!
Lorenz