On Tue, Nov 16, 2021 at 05:17:27PM -0800, Andrii Nakryiko wrote: > > + > > + rec_size = attr->core_relo_rec_size; > > + if (rec_size != sizeof(struct bpf_core_relo)) > > + return -EINVAL; > > For func_info we allow trailing zeroes (in check_btf_func, we check > MIN_BPF_FUNCINFO_SIZE and MAX_FUNCINFO_REC_SIZE). Shouldn't we do > something like that here? For both func_info and line_info the verifier indeed uses 252 as max rec_size. I believe the logic is trying to help in the following case: If llvm starts to produce a larger records the libbpf will test for the new kernel and will sanize the newer records with zeros _in place_. In case of bpf_core_relo the implementation of this patch doesn't take llvm bytes as-is. The relo records saved and copied potentially many times with every libbpf call relocations. Then later they are prepared in the final form. I don't mind doing the same 252 logic here, but when I wrote above check it felt more natural to be strict and simple, but... > > + > > + u_core_relo = make_bpfptr(attr->core_relo, uattr.is_kernel); > > + expected_size = sizeof(struct bpf_core_relo); > > + ncopy = min_t(u32, expected_size, rec_size); > > I'm confused, a few lines above you errored out if expected_size != rec_size... but then I kept this part and below bpf_check_uarg_tail_zero(). Clearly I couldn't make up my mind :) So drop all future proofing ? or do 252 ?
