| Message ID | 20230302123440.1193507-1-lmb@isovalent.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | btf: fix resolving BTF_KIND_VAR after ARRAY, STRUCT, UNION, PTR | expand |
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 <lmb@isovalent.com> > --- > 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 <martin.lau@linux.dev> 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
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);
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 <lmb@isovalent.com> --- kernel/bpf/btf.c | 1 + 1 file changed, 1 insertion(+)