| Message ID | 62cf123c51006a02c1ffa9aaaf2881a8eb292d59.1554314902.git.daniel@iogearbox.net |
|---|---|
| State | RFC |
| Headers | show |
| Series | BPF support for global data | expand |
On Wed, Apr 03, 2019 at 08:22:57PM +0200, Daniel Borkmann wrote: > This work adds kernel-side verification, logging and seq_show dumping > of BTF Var and DataSec kinds which are emitted with latest LLVM. The > following constraints apply: > > BTF Var must have: > > - Its kind_flag is 0 > - Its vlen is 0 > - Must point to a valid type > - Type must not resolve to a forward type > - Must have a valid name > - Name may include dots (e.g. in case of static variables > inside functions) > - Cannot be a member of a struct/union > - Linkage so far can either only be static or global/allocated > > BTF DataSec must have: > > - Its kind_flag is 0 > - Its vlen cannot be 0 > - Its size cannot be 0 > - Must have a valid name > - Name may include dots (e.g. to represent .bss, .data, .rodata etc) > - Cannot be a member of a struct/union > - Inner btf_var_secinfo array with {type,offset,size} triple > must be sorted by offset in ascending order > - Type must always point to BTF Var > - BTF resolved size of Var must be <= size provided by triple > - DataSec size must be >= sum of triple sizes (thus holes > are allowed) Looks very good. A few questions. [ ... ] > @@ -308,6 +320,7 @@ static bool btf_type_is_modifier(const struct btf_type *t) > case BTF_KIND_VOLATILE: > case BTF_KIND_CONST: > case BTF_KIND_RESTRICT: > + case BTF_KIND_VAR: Why BTF_KIND_VAR is added here? If possible, it may be better to keep is_modifier() as is since var intuitively does not belong to the is_modifier() test. > return true; > } > > @@ -375,13 +388,27 @@ static bool btf_type_is_int(const struct btf_type *t) > return BTF_INFO_KIND(t->info) == BTF_KIND_INT; > } > > +static bool btf_type_is_var(const struct btf_type *t) > +{ > + return BTF_INFO_KIND(t->info) == BTF_KIND_VAR; > +} > + > +static bool btf_type_is_datasec(const struct btf_type *t) > +{ > + return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC; > +} > + > /* What types need to be resolved? > * > * btf_type_is_modifier() is an obvious one. > * > * btf_type_is_struct() because its member refers to > * another type (through member->type). > - > + * > + * btf_type_is_var() because the variable refers to > + * another type. btf_type_is_datasec() holds multiple > + * btf_type_is_var() types that need resolving. > + * > * btf_type_is_array() because its element (array->type) > * refers to another type. Array can be thought of a > * special case of struct while array just has the same > @@ -390,9 +417,11 @@ static bool btf_type_is_int(const struct btf_type *t) > static bool btf_type_needs_resolve(const struct btf_type *t) > { > return btf_type_is_modifier(t) || > - btf_type_is_ptr(t) || > - btf_type_is_struct(t) || > - btf_type_is_array(t); > + btf_type_is_ptr(t) || > + btf_type_is_struct(t) || > + btf_type_is_array(t) || > + btf_type_is_var(t) || is_var() is also tested here on top of is_modifier(). > + btf_type_is_datasec(t); > } > > /* t->size can be used */ > @@ -403,6 +432,7 @@ static bool btf_type_has_size(const struct btf_type *t) > case BTF_KIND_STRUCT: > case BTF_KIND_UNION: > case BTF_KIND_ENUM: > + case BTF_KIND_DATASEC: > return true; > } > [ ... ] > +static int btf_var_resolve(struct btf_verifier_env *env, > + const struct resolve_vertex *v) > +{ > + const struct btf_type *next_type; > + const struct btf_type *t = v->t; > + u32 next_type_id = t->type; > + struct btf *btf = env->btf; > + u32 next_type_size; > + > + next_type = btf_type_by_id(btf, next_type_id); Does the BTF verifier complain if BTF_KIND_VAR -> BTF_KIND_VAR -> BTF_KIND_INT? The same goes for BTF_KIND_PTR -> BTF_KIND_VAR -> BTF_KIND_INT. > + if (!next_type) { > + btf_verifier_log_type(env, v->t, "Invalid type_id"); > + return -EINVAL; > + } > + > + if (!env_type_is_resolve_sink(env, next_type) && > + !env_type_is_resolved(env, next_type_id)) > + return env_stack_push(env, next_type, next_type_id); > + > + if (btf_type_is_modifier(next_type)) { May be a few comments on why this is needed. Together with the is_modifier() change above, it is not immediately clear to me. I suspect this could be left to be done in the btf_ptr_resolve() as well if the ptr type happens to be behind the var type in the ".BTF" ELF. > + const struct btf_type *resolved_type; > + u32 resolved_type_id; > + > + resolved_type_id = next_type_id; > + resolved_type = btf_type_id_resolve(btf, &resolved_type_id); > + > + if (btf_type_is_ptr(resolved_type) && > + !env_type_is_resolve_sink(env, resolved_type) && > + !env_type_is_resolved(env, resolved_type_id)) > + return env_stack_push(env, resolved_type, > + resolved_type_id); > + } > + > + /* We must resolve to something concrete at this point, no > + * forward types or similar that would resolve to size of > + * zero is allowed. > + */ > + if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) { > + btf_verifier_log_type(env, v->t, "Invalid type_id"); > + return -EINVAL; > + } > + > + env_stack_pop_resolved(env, next_type_id, next_type_size); > + > + return 0; > +} > + [ ... ] > @@ -2622,13 +2983,17 @@ static bool btf_resolve_valid(struct btf_verifier_env *env, > if (!env_type_is_resolved(env, type_id)) > return false; > > - if (btf_type_is_struct(t)) > + if (btf_type_is_struct(t) || btf_type_is_datasec(t)) > return !btf->resolved_ids[type_id] && > - !btf->resolved_sizes[type_id]; > + !btf->resolved_sizes[type_id]; > > - if (btf_type_is_modifier(t) || btf_type_is_ptr(t)) { > + if (btf_type_is_modifier(t) || btf_type_is_ptr(t) || > + btf_type_is_var(t)) { > t = btf_type_id_resolve(btf, &type_id); > - return t && !btf_type_is_modifier(t); > + return t && > + !btf_type_is_modifier(t) && > + !btf_type_is_var(t) && > + !btf_type_is_datasec(t); > } > > if (btf_type_is_array(t)) { > -- > 2.9.5 >
On Thu, Apr 04, 2019 at 07:20:51PM +0000, Martin Lau wrote: > On Wed, Apr 03, 2019 at 08:22:57PM +0200, Daniel Borkmann wrote: > > This work adds kernel-side verification, logging and seq_show dumping > > of BTF Var and DataSec kinds which are emitted with latest LLVM. The > > following constraints apply: > > > > BTF Var must have: > > > > - Its kind_flag is 0 > > - Its vlen is 0 > > - Must point to a valid type > > - Type must not resolve to a forward type > > - Must have a valid name > > - Name may include dots (e.g. in case of static variables > > inside functions) > > - Cannot be a member of a struct/union > > - Linkage so far can either only be static or global/allocated > > > > BTF DataSec must have: > > > > - Its kind_flag is 0 > > - Its vlen cannot be 0 > > - Its size cannot be 0 > > - Must have a valid name > > - Name may include dots (e.g. to represent .bss, .data, .rodata etc) > > - Cannot be a member of a struct/union > > - Inner btf_var_secinfo array with {type,offset,size} triple > > must be sorted by offset in ascending order > > - Type must always point to BTF Var > > - BTF resolved size of Var must be <= size provided by triple > > - DataSec size must be >= sum of triple sizes (thus holes > > are allowed) > Looks very good. A few questions. > > [ ... ] > > > @@ -308,6 +320,7 @@ static bool btf_type_is_modifier(const struct btf_type *t) > > case BTF_KIND_VOLATILE: > > case BTF_KIND_CONST: > > case BTF_KIND_RESTRICT: > > + case BTF_KIND_VAR: > Why BTF_KIND_VAR is added here? > > If possible, it may be better to keep is_modifier() as is since var > intuitively does not belong to the is_modifier() test. > > > return true; > > } > > > > @@ -375,13 +388,27 @@ static bool btf_type_is_int(const struct btf_type *t) > > return BTF_INFO_KIND(t->info) == BTF_KIND_INT; > > } > > > > +static bool btf_type_is_var(const struct btf_type *t) > > +{ > > + return BTF_INFO_KIND(t->info) == BTF_KIND_VAR; > > +} > > + > > +static bool btf_type_is_datasec(const struct btf_type *t) > > +{ > > + return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC; > > +} > > + > > /* What types need to be resolved? > > * > > * btf_type_is_modifier() is an obvious one. > > * > > * btf_type_is_struct() because its member refers to > > * another type (through member->type). > > - > > + * > > + * btf_type_is_var() because the variable refers to > > + * another type. btf_type_is_datasec() holds multiple > > + * btf_type_is_var() types that need resolving. > > + * > > * btf_type_is_array() because its element (array->type) > > * refers to another type. Array can be thought of a > > * special case of struct while array just has the same > > @@ -390,9 +417,11 @@ static bool btf_type_is_int(const struct btf_type *t) > > static bool btf_type_needs_resolve(const struct btf_type *t) > > { > > return btf_type_is_modifier(t) || > > - btf_type_is_ptr(t) || > > - btf_type_is_struct(t) || > > - btf_type_is_array(t); > > + btf_type_is_ptr(t) || > > + btf_type_is_struct(t) || > > + btf_type_is_array(t) || > > + btf_type_is_var(t) || > is_var() is also tested here on top of is_modifier(). > > > + btf_type_is_datasec(t); > > } > > > > /* t->size can be used */ > > @@ -403,6 +432,7 @@ static bool btf_type_has_size(const struct btf_type *t) > > case BTF_KIND_STRUCT: > > case BTF_KIND_UNION: > > case BTF_KIND_ENUM: > > + case BTF_KIND_DATASEC: > > return true; > > } > > > > [ ... ] > > > +static int btf_var_resolve(struct btf_verifier_env *env, > > + const struct resolve_vertex *v) > > +{ > > + const struct btf_type *next_type; > > + const struct btf_type *t = v->t; > > + u32 next_type_id = t->type; > > + struct btf *btf = env->btf; > > + u32 next_type_size; > > + > > + next_type = btf_type_by_id(btf, next_type_id); > Does the BTF verifier complain if BTF_KIND_VAR -> BTF_KIND_VAR -> BTF_KIND_INT? > > The same goes for BTF_KIND_PTR -> BTF_KIND_VAR -> BTF_KIND_INT. I was about to try but it seems I cannot apply the set cleanly. Likely due to some recent changes. After a quick skim through, the above cases seem possible. If rejecting the above cases is desired, I think it may be easier to check them at the individual type's _resolve() instead of depending on the final resort btf_resolve_valid(). The individual type's _resolve() should know better what are the acceptable next_type[s]. I was thinking btf_resolve_valid() is more like a "btf verifier internal error" to ensure the earlier individual type's _resolve() is sane. It seems at least the modifier_resolve() and ptr_resolve() are missing to reject BTF_KIND_FUNC. I will code up a patch to fix BTF_KIND_FUNC. > > > + if (!next_type) { > > + btf_verifier_log_type(env, v->t, "Invalid type_id"); > > + return -EINVAL; > > + } > > + > > + if (!env_type_is_resolve_sink(env, next_type) && > > + !env_type_is_resolved(env, next_type_id)) > > + return env_stack_push(env, next_type, next_type_id); > > + > > + if (btf_type_is_modifier(next_type)) { > May be a few comments on why this is needed. Together with > the is_modifier() change above, it is not immediately clear to me. > > I suspect this could be left to be done in the btf_ptr_resolve() > as well if the ptr type happens to be behind the var type in > the ".BTF" ELF. > > > + const struct btf_type *resolved_type; > > + u32 resolved_type_id; > > + > > + resolved_type_id = next_type_id; > > + resolved_type = btf_type_id_resolve(btf, &resolved_type_id); > > + > > + if (btf_type_is_ptr(resolved_type) && > > + !env_type_is_resolve_sink(env, resolved_type) && > > + !env_type_is_resolved(env, resolved_type_id)) > > + return env_stack_push(env, resolved_type, > > + resolved_type_id); > > + } > > + > > + /* We must resolve to something concrete at this point, no > > + * forward types or similar that would resolve to size of > > + * zero is allowed. > > + */ > > + if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) { > > + btf_verifier_log_type(env, v->t, "Invalid type_id"); > > + return -EINVAL; > > + } > > + > > + env_stack_pop_resolved(env, next_type_id, next_type_size); > > + > > + return 0; > > +} > > + > > [ ... ] > > > @@ -2622,13 +2983,17 @@ static bool btf_resolve_valid(struct btf_verifier_env *env, > > if (!env_type_is_resolved(env, type_id)) > > return false; > > > > - if (btf_type_is_struct(t)) > > + if (btf_type_is_struct(t) || btf_type_is_datasec(t)) > > return !btf->resolved_ids[type_id] && > > - !btf->resolved_sizes[type_id]; > > + !btf->resolved_sizes[type_id]; > > > > - if (btf_type_is_modifier(t) || btf_type_is_ptr(t)) { > > + if (btf_type_is_modifier(t) || btf_type_is_ptr(t) || > > + btf_type_is_var(t)) { > > t = btf_type_id_resolve(btf, &type_id); > > - return t && !btf_type_is_modifier(t); > > + return t && > > + !btf_type_is_modifier(t) && > > + !btf_type_is_var(t) && > > + !btf_type_is_datasec(t); > > } > > > > if (btf_type_is_array(t)) { > > -- > > 2.9.5 > >
Hey Martin, Thanks a lot for the feedback, much appreciated! On 04/05/2019 09:03 AM, Martin Lau wrote: > On Thu, Apr 04, 2019 at 07:20:51PM +0000, Martin Lau wrote: >> On Wed, Apr 03, 2019 at 08:22:57PM +0200, Daniel Borkmann wrote: >>> This work adds kernel-side verification, logging and seq_show dumping >>> of BTF Var and DataSec kinds which are emitted with latest LLVM. The >>> following constraints apply: >>> >>> BTF Var must have: >>> >>> - Its kind_flag is 0 >>> - Its vlen is 0 >>> - Must point to a valid type >>> - Type must not resolve to a forward type >>> - Must have a valid name >>> - Name may include dots (e.g. in case of static variables >>> inside functions) >>> - Cannot be a member of a struct/union >>> - Linkage so far can either only be static or global/allocated >>> >>> BTF DataSec must have: >>> >>> - Its kind_flag is 0 >>> - Its vlen cannot be 0 >>> - Its size cannot be 0 >>> - Must have a valid name >>> - Name may include dots (e.g. to represent .bss, .data, .rodata etc) >>> - Cannot be a member of a struct/union >>> - Inner btf_var_secinfo array with {type,offset,size} triple >>> must be sorted by offset in ascending order >>> - Type must always point to BTF Var >>> - BTF resolved size of Var must be <= size provided by triple >>> - DataSec size must be >= sum of triple sizes (thus holes >>> are allowed) >> Looks very good. A few questions. >> >> [ ... ] >> >>> @@ -308,6 +320,7 @@ static bool btf_type_is_modifier(const struct btf_type *t) >>> case BTF_KIND_VOLATILE: >>> case BTF_KIND_CONST: >>> case BTF_KIND_RESTRICT: >>> + case BTF_KIND_VAR: >> Why BTF_KIND_VAR is added here? >> >> If possible, it may be better to keep is_modifier() as is since var >> intuitively does not belong to the is_modifier() test. Hmm, agree, it's probably better to keep btf_type_is_modifier() as-is, I might have been mislead by the comment in that function. Need to double check whether that could trigger the WARN in btf_type_id_size(). >>> return true; >>> } >>> >>> @@ -375,13 +388,27 @@ static bool btf_type_is_int(const struct btf_type *t) >>> return BTF_INFO_KIND(t->info) == BTF_KIND_INT; >>> } >>> >>> +static bool btf_type_is_var(const struct btf_type *t) >>> +{ >>> + return BTF_INFO_KIND(t->info) == BTF_KIND_VAR; >>> +} >>> + >>> +static bool btf_type_is_datasec(const struct btf_type *t) >>> +{ >>> + return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC; >>> +} >>> + >>> /* What types need to be resolved? >>> * >>> * btf_type_is_modifier() is an obvious one. >>> * >>> * btf_type_is_struct() because its member refers to >>> * another type (through member->type). >>> - >>> + * >>> + * btf_type_is_var() because the variable refers to >>> + * another type. btf_type_is_datasec() holds multiple >>> + * btf_type_is_var() types that need resolving. >>> + * >>> * btf_type_is_array() because its element (array->type) >>> * refers to another type. Array can be thought of a >>> * special case of struct while array just has the same >>> @@ -390,9 +417,11 @@ static bool btf_type_is_int(const struct btf_type *t) >>> static bool btf_type_needs_resolve(const struct btf_type *t) >>> { >>> return btf_type_is_modifier(t) || >>> - btf_type_is_ptr(t) || >>> - btf_type_is_struct(t) || >>> - btf_type_is_array(t); >>> + btf_type_is_ptr(t) || >>> + btf_type_is_struct(t) || >>> + btf_type_is_array(t) || >>> + btf_type_is_var(t) || >> is_var() is also tested here on top of is_modifier(). Yeah, correct. >>> + btf_type_is_datasec(t); >>> } >>> >>> /* t->size can be used */ >>> @@ -403,6 +432,7 @@ static bool btf_type_has_size(const struct btf_type *t) >>> case BTF_KIND_STRUCT: >>> case BTF_KIND_UNION: >>> case BTF_KIND_ENUM: >>> + case BTF_KIND_DATASEC: >>> return true; >>> } >>> >> >> [ ... ] >> >>> +static int btf_var_resolve(struct btf_verifier_env *env, >>> + const struct resolve_vertex *v) >>> +{ >>> + const struct btf_type *next_type; >>> + const struct btf_type *t = v->t; >>> + u32 next_type_id = t->type; >>> + struct btf *btf = env->btf; >>> + u32 next_type_size; >>> + >>> + next_type = btf_type_by_id(btf, next_type_id); >> Does the BTF verifier complain if BTF_KIND_VAR -> BTF_KIND_VAR -> BTF_KIND_INT? >> >> The same goes for BTF_KIND_PTR -> BTF_KIND_VAR -> BTF_KIND_INT. > I was about to try but it seems I cannot apply the set cleanly. > Likely due to some recent changes. > > After a quick skim through, the above cases seem possible. The tests currently only check whether we end up at BTF_KIND_VAR or BTF_KIND_DATASEC, but as far as I'm aware would accept some intermediate BTF_KIND_VAR type, which should be better rejected instead. > If rejecting the above cases is desired, > I think it may be easier to check them at the individual > type's _resolve() instead of depending on the final resort > btf_resolve_valid(). The individual type's _resolve() should > know better what are the acceptable next_type[s]. Makes sense, I'll change that today and add more test coverage for this particular case. Moving the check into individual type's _resolve() handler is indeed probably the best we can do at this point, though unfortunate duplication. > I was thinking btf_resolve_valid() is more like a > "btf verifier internal error" to ensure the > earlier individual type's _resolve() is sane. > > It seems at least the modifier_resolve() and ptr_resolve() > are missing to reject BTF_KIND_FUNC. I will code > up a patch to fix BTF_KIND_FUNC. Okay. >>> + if (!next_type) { >>> + btf_verifier_log_type(env, v->t, "Invalid type_id"); >>> + return -EINVAL; >>> + } >>> + >>> + if (!env_type_is_resolve_sink(env, next_type) && >>> + !env_type_is_resolved(env, next_type_id)) >>> + return env_stack_push(env, next_type, next_type_id); >>> + >>> + if (btf_type_is_modifier(next_type)) { >> May be a few comments on why this is needed. Together with >> the is_modifier() change above, it is not immediately clear to me. Yep, the change in is_modifier() needs to be dropped. >> I suspect this could be left to be done in the btf_ptr_resolve() >> as well if the ptr type happens to be behind the var type in >> the ".BTF" ELF. >> >>> + const struct btf_type *resolved_type; >>> + u32 resolved_type_id; >>> + >>> + resolved_type_id = next_type_id; >>> + resolved_type = btf_type_id_resolve(btf, &resolved_type_id); >>> + >>> + if (btf_type_is_ptr(resolved_type) && >>> + !env_type_is_resolve_sink(env, resolved_type) && >>> + !env_type_is_resolved(env, resolved_type_id)) >>> + return env_stack_push(env, resolved_type, >>> + resolved_type_id); >>> + } >>> + >>> + /* We must resolve to something concrete at this point, no >>> + * forward types or similar that would resolve to size of >>> + * zero is allowed. >>> + */ >>> + if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) { >>> + btf_verifier_log_type(env, v->t, "Invalid type_id"); >>> + return -EINVAL; >>> + } >>> + >>> + env_stack_pop_resolved(env, next_type_id, next_type_size); >>> + >>> + return 0; >>> +} >>> + >> >> [ ... ] >> >>> @@ -2622,13 +2983,17 @@ static bool btf_resolve_valid(struct btf_verifier_env *env, >>> if (!env_type_is_resolved(env, type_id)) >>> return false; >>> >>> - if (btf_type_is_struct(t)) >>> + if (btf_type_is_struct(t) || btf_type_is_datasec(t)) >>> return !btf->resolved_ids[type_id] && >>> - !btf->resolved_sizes[type_id]; >>> + !btf->resolved_sizes[type_id]; >>> >>> - if (btf_type_is_modifier(t) || btf_type_is_ptr(t)) { >>> + if (btf_type_is_modifier(t) || btf_type_is_ptr(t) || >>> + btf_type_is_var(t)) { >>> t = btf_type_id_resolve(btf, &type_id); >>> - return t && !btf_type_is_modifier(t); >>> + return t && >>> + !btf_type_is_modifier(t) && >>> + !btf_type_is_var(t) && >>> + !btf_type_is_datasec(t); >>> } >>> >>> if (btf_type_is_array(t)) { >>> -- >>> 2.9.5 >>>
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index bd3921b..5c9285c 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -185,6 +185,16 @@ i < btf_type_vlen(struct_type); \ i++, member++) +#define for_each_vsi(i, struct_type, member) \ + for (i = 0, member = btf_type_var_secinfo(struct_type); \ + i < btf_type_vlen(struct_type); \ + i++, member++) + +#define for_each_vsi_from(i, from, struct_type, member) \ + for (i = from, member = btf_type_var_secinfo(struct_type) + from; \ + i < btf_type_vlen(struct_type); \ + i++, member++) + static DEFINE_IDR(btf_idr); static DEFINE_SPINLOCK(btf_idr_lock); @@ -262,6 +272,8 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = { [BTF_KIND_RESTRICT] = "RESTRICT", [BTF_KIND_FUNC] = "FUNC", [BTF_KIND_FUNC_PROTO] = "FUNC_PROTO", + [BTF_KIND_VAR] = "VAR", + [BTF_KIND_DATASEC] = "DATASEC", }; struct btf_kind_operations { @@ -308,6 +320,7 @@ static bool btf_type_is_modifier(const struct btf_type *t) case BTF_KIND_VOLATILE: case BTF_KIND_CONST: case BTF_KIND_RESTRICT: + case BTF_KIND_VAR: return true; } @@ -375,13 +388,27 @@ static bool btf_type_is_int(const struct btf_type *t) return BTF_INFO_KIND(t->info) == BTF_KIND_INT; } +static bool btf_type_is_var(const struct btf_type *t) +{ + return BTF_INFO_KIND(t->info) == BTF_KIND_VAR; +} + +static bool btf_type_is_datasec(const struct btf_type *t) +{ + return BTF_INFO_KIND(t->info) == BTF_KIND_DATASEC; +} + /* What types need to be resolved? * * btf_type_is_modifier() is an obvious one. * * btf_type_is_struct() because its member refers to * another type (through member->type). - + * + * btf_type_is_var() because the variable refers to + * another type. btf_type_is_datasec() holds multiple + * btf_type_is_var() types that need resolving. + * * btf_type_is_array() because its element (array->type) * refers to another type. Array can be thought of a * special case of struct while array just has the same @@ -390,9 +417,11 @@ static bool btf_type_is_int(const struct btf_type *t) static bool btf_type_needs_resolve(const struct btf_type *t) { return btf_type_is_modifier(t) || - btf_type_is_ptr(t) || - btf_type_is_struct(t) || - btf_type_is_array(t); + btf_type_is_ptr(t) || + btf_type_is_struct(t) || + btf_type_is_array(t) || + btf_type_is_var(t) || + btf_type_is_datasec(t); } /* t->size can be used */ @@ -403,6 +432,7 @@ static bool btf_type_has_size(const struct btf_type *t) case BTF_KIND_STRUCT: case BTF_KIND_UNION: case BTF_KIND_ENUM: + case BTF_KIND_DATASEC: return true; } @@ -467,6 +497,16 @@ static const struct btf_enum *btf_type_enum(const struct btf_type *t) return (const struct btf_enum *)(t + 1); } +static const struct btf_var *btf_type_var(const struct btf_type *t) +{ + return (const struct btf_var *)(t + 1); +} + +static const struct btf_var_secinfo *btf_type_var_secinfo(const struct btf_type *t) +{ + return (const struct btf_var_secinfo *)(t + 1); +} + static const struct btf_kind_operations *btf_type_ops(const struct btf_type *t) { return kind_ops[BTF_INFO_KIND(t->info)]; @@ -478,23 +518,31 @@ static bool btf_name_offset_valid(const struct btf *btf, u32 offset) offset < btf->hdr.str_len; } -/* Only C-style identifier is permitted. This can be relaxed if - * necessary. - */ -static bool btf_name_valid_identifier(const struct btf *btf, u32 offset) +static bool __btf_name_char_ok(char c, bool first, bool dot_ok) +{ + if ((first ? !isalpha(c) : + !isalnum(c)) && + c != '_' && + ((c == '.' && !dot_ok) || + c != '.')) + return false; + return true; +} + +static bool __btf_name_valid(const struct btf *btf, u32 offset, bool dot_ok) { /* offset must be valid */ const char *src = &btf->strings[offset]; const char *src_limit; - if (!isalpha(*src) && *src != '_') + if (!__btf_name_char_ok(*src, true, dot_ok)) return false; /* set a limit on identifier length */ src_limit = src + KSYM_NAME_LEN; src++; while (*src && src < src_limit) { - if (!isalnum(*src) && *src != '_') + if (!__btf_name_char_ok(*src, false, dot_ok)) return false; src++; } @@ -502,6 +550,19 @@ static bool btf_name_valid_identifier(const struct btf *btf, u32 offset) return !*src; } +/* Only C-style identifier is permitted. This can be relaxed if + * necessary. + */ +static bool btf_name_valid_identifier(const struct btf *btf, u32 offset) +{ + return __btf_name_valid(btf, offset, false); +} + +static bool btf_name_valid_section(const struct btf *btf, u32 offset) +{ + return __btf_name_valid(btf, offset, true); +} + static const char *__btf_name_by_offset(const struct btf *btf, u32 offset) { if (!offset) @@ -697,6 +758,32 @@ static void btf_verifier_log_member(struct btf_verifier_env *env, __btf_verifier_log(log, "\n"); } +__printf(4, 5) +static void btf_verifier_log_vsi(struct btf_verifier_env *env, + const struct btf_type *datasec_type, + const struct btf_var_secinfo *vsi, + const char *fmt, ...) +{ + struct bpf_verifier_log *log = &env->log; + va_list args; + + if (!bpf_verifier_log_needed(log)) + return; + if (env->phase != CHECK_META) + btf_verifier_log_type(env, datasec_type, NULL); + + __btf_verifier_log(log, "\t type_id=%u offset=%u size=%u", + vsi->type, vsi->offset, vsi->size); + if (fmt && *fmt) { + __btf_verifier_log(log, " "); + va_start(args, fmt); + bpf_verifier_vlog(log, fmt, args); + va_end(args); + } + + __btf_verifier_log(log, "\n"); +} + static void btf_verifier_log_hdr(struct btf_verifier_env *env, u32 btf_data_size) { @@ -1542,6 +1629,53 @@ static int btf_modifier_resolve(struct btf_verifier_env *env, return 0; } +static int btf_var_resolve(struct btf_verifier_env *env, + const struct resolve_vertex *v) +{ + const struct btf_type *next_type; + const struct btf_type *t = v->t; + u32 next_type_id = t->type; + struct btf *btf = env->btf; + u32 next_type_size; + + next_type = btf_type_by_id(btf, next_type_id); + if (!next_type) { + btf_verifier_log_type(env, v->t, "Invalid type_id"); + return -EINVAL; + } + + if (!env_type_is_resolve_sink(env, next_type) && + !env_type_is_resolved(env, next_type_id)) + return env_stack_push(env, next_type, next_type_id); + + if (btf_type_is_modifier(next_type)) { + const struct btf_type *resolved_type; + u32 resolved_type_id; + + resolved_type_id = next_type_id; + resolved_type = btf_type_id_resolve(btf, &resolved_type_id); + + if (btf_type_is_ptr(resolved_type) && + !env_type_is_resolve_sink(env, resolved_type) && + !env_type_is_resolved(env, resolved_type_id)) + return env_stack_push(env, resolved_type, + resolved_type_id); + } + + /* We must resolve to something concrete at this point, no + * forward types or similar that would resolve to size of + * zero is allowed. + */ + if (!btf_type_id_size(btf, &next_type_id, &next_type_size)) { + btf_verifier_log_type(env, v->t, "Invalid type_id"); + return -EINVAL; + } + + env_stack_pop_resolved(env, next_type_id, next_type_size); + + return 0; +} + static int btf_ptr_resolve(struct btf_verifier_env *env, const struct resolve_vertex *v) { @@ -1609,6 +1743,15 @@ static void btf_modifier_seq_show(const struct btf *btf, btf_type_ops(t)->seq_show(btf, t, type_id, data, bits_offset, m); } +static void btf_var_seq_show(const struct btf *btf, const struct btf_type *t, + u32 type_id, void *data, u8 bits_offset, + struct seq_file *m) +{ + t = btf_type_id_resolve(btf, &type_id); + + btf_type_ops(t)->seq_show(btf, t, type_id, data, bits_offset, m); +} + static void btf_ptr_seq_show(const struct btf *btf, const struct btf_type *t, u32 type_id, void *data, u8 bits_offset, struct seq_file *m) @@ -2411,6 +2554,222 @@ static struct btf_kind_operations func_ops = { .seq_show = btf_df_seq_show, }; +static s32 btf_var_check_meta(struct btf_verifier_env *env, + const struct btf_type *t, + u32 meta_left) +{ + const struct btf_var *var; + u32 meta_needed = sizeof(*var); + + if (meta_left < meta_needed) { + btf_verifier_log_basic(env, t, + "meta_left:%u meta_needed:%u", + meta_left, meta_needed); + return -EINVAL; + } + + if (btf_type_vlen(t)) { + btf_verifier_log_type(env, t, "vlen != 0"); + return -EINVAL; + } + + if (btf_type_kflag(t)) { + btf_verifier_log_type(env, t, "Invalid btf_info kind_flag"); + return -EINVAL; + } + + if (!t->name_off || + !__btf_name_valid(env->btf, t->name_off, true)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + + /* A var cannot be in type void */ + if (!t->type || !BTF_TYPE_ID_VALID(t->type)) { + btf_verifier_log_type(env, t, "Invalid type_id"); + return -EINVAL; + } + + var = btf_type_var(t); + if (var->linkage != BTF_VAR_STATIC && + var->linkage != BTF_VAR_GLOBAL_ALLOCATED) { + btf_verifier_log_type(env, t, "Linkage not supported"); + return -EINVAL; + } + + btf_verifier_log_type(env, t, NULL); + + return meta_needed; +} + +static void btf_var_log(struct btf_verifier_env *env, const struct btf_type *t) +{ + const struct btf_var *var = btf_type_var(t); + + btf_verifier_log(env, "type_id=%u linkage=%u", t->type, var->linkage); +} + +static const struct btf_kind_operations var_ops = { + .check_meta = btf_var_check_meta, + .resolve = btf_var_resolve, + .check_member = btf_df_check_member, + .check_kflag_member = btf_df_check_kflag_member, + .log_details = btf_var_log, + .seq_show = btf_var_seq_show, +}; + +static s32 btf_datasec_check_meta(struct btf_verifier_env *env, + const struct btf_type *t, + u32 meta_left) +{ + const struct btf_var_secinfo *vsi; + u64 last_vsi_end_off = 0, sum = 0; + u32 i, meta_needed; + + meta_needed = btf_type_vlen(t) * sizeof(*vsi); + if (meta_left < meta_needed) { + btf_verifier_log_basic(env, t, + "meta_left:%u meta_needed:%u", + meta_left, meta_needed); + return -EINVAL; + } + + if (!btf_type_vlen(t)) { + btf_verifier_log_type(env, t, "vlen == 0"); + return -EINVAL; + } + + if (!t->size) { + btf_verifier_log_type(env, t, "size == 0"); + return -EINVAL; + } + + if (btf_type_kflag(t)) { + btf_verifier_log_type(env, t, "Invalid btf_info kind_flag"); + return -EINVAL; + } + + if (!t->name_off || + !btf_name_valid_section(env->btf, t->name_off)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + + btf_verifier_log_type(env, t, NULL); + + for_each_vsi(i, t, vsi) { + /* A var cannot be in type void */ + if (!vsi->type || !BTF_TYPE_ID_VALID(vsi->type)) { + btf_verifier_log_vsi(env, t, vsi, + "Invalid type_id"); + return -EINVAL; + } + + if (vsi->offset < last_vsi_end_off || vsi->offset >= t->size) { + btf_verifier_log_vsi(env, t, vsi, + "Invalid offset"); + return -EINVAL; + } + + if (!vsi->size || vsi->size > t->size) { + btf_verifier_log_vsi(env, t, vsi, + "Invalid size"); + return -EINVAL; + } + + last_vsi_end_off = vsi->offset + vsi->size; + if (last_vsi_end_off > t->size) { + btf_verifier_log_vsi(env, t, vsi, + "Invalid offset+size"); + return -EINVAL; + } + + btf_verifier_log_vsi(env, t, vsi, NULL); + sum += vsi->size; + } + + if (t->size < sum) { + btf_verifier_log_type(env, t, "Invalid btf_info size"); + return -EINVAL; + } + + return meta_needed; +} + +static int btf_datasec_resolve(struct btf_verifier_env *env, + const struct resolve_vertex *v) +{ + const struct btf_var_secinfo *vsi; + struct btf *btf = env->btf; + u16 i; + + for_each_vsi_from(i, v->next_member, v->t, vsi) { + u32 var_type_id = vsi->type, type_id, type_size = 0; + const struct btf_type *var_type = btf_type_by_id(env->btf, + var_type_id); + if (!var_type || !btf_type_is_var(var_type)) { + btf_verifier_log_vsi(env, v->t, vsi, + "Not a VAR kind member"); + return -EINVAL; + } + + 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); + } + + type_id = var_type->type; + if (!btf_type_id_size(btf, &type_id, &type_size)) { + btf_verifier_log_vsi(env, v->t, vsi, "Invalid type"); + return -EINVAL; + } + + if (vsi->size < type_size) { + btf_verifier_log_vsi(env, v->t, vsi, "Invalid size"); + return -EINVAL; + } + } + + env_stack_pop_resolved(env, 0, 0); + return 0; +} + +static void btf_datasec_log(struct btf_verifier_env *env, + const struct btf_type *t) +{ + btf_verifier_log(env, "size=%u vlen=%u", t->size, btf_type_vlen(t)); +} + +static void btf_datasec_seq_show(const struct btf *btf, + const struct btf_type *t, u32 type_id, + void *data, u8 bits_offset, + struct seq_file *m) +{ + const struct btf_var_secinfo *vsi; + const struct btf_type *var; + u32 i; + + seq_printf(m, "section (\"%s\") = {", __btf_name_by_offset(btf, t->name_off)); + for_each_vsi(i, t, vsi) { + var = btf_type_by_id(btf, vsi->type); + if (i) + seq_puts(m, ","); + btf_type_ops(var)->seq_show(btf, var, vsi->type, + data + vsi->offset, bits_offset, m); + } + seq_puts(m, "}"); +} + +static const struct btf_kind_operations datasec_ops = { + .check_meta = btf_datasec_check_meta, + .resolve = btf_datasec_resolve, + .check_member = btf_df_check_member, + .check_kflag_member = btf_df_check_kflag_member, + .log_details = btf_datasec_log, + .seq_show = btf_datasec_seq_show, +}; + static int btf_func_proto_check(struct btf_verifier_env *env, const struct btf_type *t) { @@ -2542,6 +2901,8 @@ static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS] = { [BTF_KIND_RESTRICT] = &modifier_ops, [BTF_KIND_FUNC] = &func_ops, [BTF_KIND_FUNC_PROTO] = &func_proto_ops, + [BTF_KIND_VAR] = &var_ops, + [BTF_KIND_DATASEC] = &datasec_ops, }; static s32 btf_check_meta(struct btf_verifier_env *env, @@ -2622,13 +2983,17 @@ static bool btf_resolve_valid(struct btf_verifier_env *env, if (!env_type_is_resolved(env, type_id)) return false; - if (btf_type_is_struct(t)) + if (btf_type_is_struct(t) || btf_type_is_datasec(t)) return !btf->resolved_ids[type_id] && - !btf->resolved_sizes[type_id]; + !btf->resolved_sizes[type_id]; - if (btf_type_is_modifier(t) || btf_type_is_ptr(t)) { + if (btf_type_is_modifier(t) || btf_type_is_ptr(t) || + btf_type_is_var(t)) { t = btf_type_id_resolve(btf, &type_id); - return t && !btf_type_is_modifier(t); + return t && + !btf_type_is_modifier(t) && + !btf_type_is_var(t) && + !btf_type_is_datasec(t); } if (btf_type_is_array(t)) {
This work adds kernel-side verification, logging and seq_show dumping of BTF Var and DataSec kinds which are emitted with latest LLVM. The following constraints apply: BTF Var must have: - Its kind_flag is 0 - Its vlen is 0 - Must point to a valid type - Type must not resolve to a forward type - Must have a valid name - Name may include dots (e.g. in case of static variables inside functions) - Cannot be a member of a struct/union - Linkage so far can either only be static or global/allocated BTF DataSec must have: - Its kind_flag is 0 - Its vlen cannot be 0 - Its size cannot be 0 - Must have a valid name - Name may include dots (e.g. to represent .bss, .data, .rodata etc) - Cannot be a member of a struct/union - Inner btf_var_secinfo array with {type,offset,size} triple must be sorted by offset in ascending order - Type must always point to BTF Var - BTF resolved size of Var must be <= size provided by triple - DataSec size must be >= sum of triple sizes (thus holes are allowed) Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> --- kernel/bpf/btf.c | 393 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 379 insertions(+), 14 deletions(-)