Most ABIs allow functions to receive structs passed by value, if they fit in a register or a pair of registers, depending on the exact ABI. However, when there is a struct passed by value but all registers are already used for parameters passing, the struct is still passed by value but on the stack. This becomes an issue if the passed struct is defined with some attributes like __attribute__((packed)) or __attribute__((aligned(X)), as its location on the stack is altered, but this change is not reflected in dwarf information. The corresponding BTF data generated from this can lead to incorrect BPF trampolines generation (eg to attach bpf tracing programs to kernel functions) in the Linux kernel. Prevent those wrong cases by not encoding functions consuming structs passed by value on stack, when those structs do not have the expected alignment due to some attribute usage. Signed-off-by: Alexis Lothoré (eBPF Foundation) <alexis.lothore@xxxxxxxxxxx> --- Changes in v2: - do not deny any struct passed by value, only those passed on stack AND with some attribute alteration - use the existing class__infer_packed_attributes to deduce is a struct is "altered". As a consequence, move the function filtering from parameter__new to btf_encoder__encode_cu, to make sure that all the needed data has been parsed from debug info --- btf_encoder.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++-- dwarves.h | 1 + 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/btf_encoder.c b/btf_encoder.c index 0bc23349b5d740c3ddab8208b2e15cdbdd139b9d..16739066caae808aea77175e6c221afbe37b7c70 100644 --- a/btf_encoder.c +++ b/btf_encoder.c @@ -87,6 +87,7 @@ struct btf_encoder_func_state { uint8_t optimized_parms:1; uint8_t unexpected_reg:1; uint8_t inconsistent_proto:1; + uint8_t uncertain_parm_loc:1; int ret_type_id; struct btf_encoder_func_parm *parms; struct btf_encoder_func_annot *annots; @@ -1203,6 +1204,7 @@ static int32_t btf_encoder__save_func(struct btf_encoder *encoder, struct functi state->inconsistent_proto = ftype->inconsistent_proto; state->unexpected_reg = ftype->unexpected_reg; state->optimized_parms = ftype->optimized_parms; + state->uncertain_parm_loc = ftype->uncertain_parm_loc; ftype__for_each_parameter(ftype, param) { const char *name = parameter__name(param) ?: ""; @@ -1430,9 +1432,15 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e /* do not exclude functions with optimized-out parameters; they * may still be _called_ with the right parameter values, they * just do not _use_ them. Only exclude functions with - * unexpected register use or multiple inconsistent prototypes. + * unexpected register use, multiple inconsistent prototypes or + * uncertain parameters location */ - add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto; + add_to_btf |= !state->unexpected_reg && !state->inconsistent_proto && !state->uncertain_parm_loc; + + if (state->uncertain_parm_loc) + btf_encoder__log_func_skip(encoder, saved_fns[i].elf, + "uncertain parameter location\n", + 0, 0); if (add_to_btf) { err = btf_encoder__add_func(state->encoder, state); @@ -2553,6 +2561,39 @@ void btf_encoder__delete(struct btf_encoder *encoder) free(encoder); } +static bool ftype__has_uncertain_arg_loc(struct cu *cu, struct ftype *ftype) +{ + struct parameter *param; + int param_idx = 0; + + if (ftype->nr_parms < cu->nr_register_params) + return false; + + ftype__for_each_parameter(ftype, param) { + if (param_idx++ < cu->nr_register_params) + continue; + + struct tag *type = cu__type(cu, param->tag.type); + + if (type == NULL || !tag__is_struct(type)) + continue; + + struct type *ctype = tag__type(type); + if (ctype->namespace.name == 0) + continue; + + struct class *class = tag__class(type); + + class__find_holes(class); + class__infer_packed_attributes(class, cu); + + if (class->is_packed) + return true; + } + + return false; +} + int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct conf_load *conf_load) { struct llvm_annotation *annot; @@ -2647,6 +2688,8 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co * Skip functions that: * - are marked as declarations * - do not have full argument names + * - have arguments with uncertain locations, e.g packed + * structs passed by value on stack * - are not in ftrace list (if it's available) * - are not external (in case ftrace filter is not available) */ @@ -2693,6 +2736,9 @@ int btf_encoder__encode_cu(struct btf_encoder *encoder, struct cu *cu, struct co if (!func) continue; + if (ftype__has_uncertain_arg_loc(cu, &fn->proto)) + fn->proto.uncertain_parm_loc = 1; + err = btf_encoder__save_func(encoder, fn, func); if (err) goto out; diff --git a/dwarves.h b/dwarves.h index 36c689847ebf29a1ab9936f9d0f928dd46514547..d689aee5910f4b40dc13b3e9dc596dfbe6a2c3d0 100644 --- a/dwarves.h +++ b/dwarves.h @@ -1021,6 +1021,7 @@ struct ftype { uint8_t unexpected_reg:1; uint8_t processed:1; uint8_t inconsistent_proto:1; + uint8_t uncertain_parm_loc:1; struct list_head template_type_params; struct list_head template_value_params; struct template_parameter_pack *template_parameter_pack; -- 2.50.0