On 21/11/2023 19:44, Andrii Nakryiko wrote: > On Tue, Nov 14, 2023 at 12:20 PM Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: >> >> hi folks >> >> I wanted to capture feedback received on the approach described here for >> BTF module generation at my talk at LPC [1]. >> >> Stepping back, the aim is to provide a way to generate BTF for a module >> such that it is somewhat resilient to minor changes in underlying BTF, >> so it does not have to be rebuilt every time vmlinux is built. The >> module references to vmlinux BTF ids are currently very brittle, and >> even for the same kernel we get different vmlinux BTF ids if the BTF is >> rebuilt. So the aim is to support a more robust method of module BTF >> generation. Note that the approach described here is not needed for >> modules that are built at the same time as the kernel, so it's unlikely >> any in-tree modules will need this, but it will be useful for cases such >> as where modules are delivered via a package and want to make use >> of BTF such that it will not be invalidated. >> >> Turning to the talk, the general consensus - I think - was that the >> standalone BTF approach described in this series was problematic. >> Consider kfuncs, if we have, for example, our own definition of a >> structure in standalone module BTF, the BTF id of the local structure >> will not match that of the core kernel, which has the potential to >> confuse the verifier. >> >> A similar problem exists for tracing; we would trace an sk_buff in >> the module via the module's view of struct sk_buff, but we have no >> guarantees that the module's view is still consistent with the vmlinux >> representation (which actually allocated it). >> >> Hopefully I've characterized this correctly; let me know if I missed >> something here. > > Correct. > >> >> So we need some means to both remap BTF ids in the module BTF that refer >> to the vmlinux BTF so they point at the right types, _and_ to check the >> consistency of the representation of a vmlinux type between module BTF >> build time and when it is loaded into the kernel. >> >> With this in mind, I think a good way forward might be something like >> the following: >> >> For cases where we want more change-independent module BTF - which >> is resilient to things like reshuffling of vmlinux BTF ids, and small >> changes that don't invalidate structure use completely - we add >> a "relocatable" option to the --btf_features list of features for pahole >> encoding of module BTF. >> >> This option would not be needed for modules built at the same time as >> the kernel, since the BTF ids and the types they refer to are consistent. >> >> When used however, it would tell BTF dedup in pahole to add reocation >> information as well as generating usual split BTF at the time of module >> BTF generation. This relocation information would consist of >> descriptions of the BTF types that the module refers to in base BTF and >> their dependents. By providing such descriptions, we can then reconcile >> the views of types between module and kernel, or if such reconciliation >> is impossible, we can refuse to use the BTF. The amount of information >> needed for a module will need to be determined, but I'm hopeful in most >> cases it would be a small subset of the type information >> required for vmlinux as a whole. >> >> The process of reconciling module and vmlinux BTF at module load time >> would then be >> >> 1. Remap all the split BTF ids representing module-specific types >> and functions to start at last_vmlinux_id + 1. Since the current >> vmlinux may have a different number of types than the vmlinux >> at time of encoding, this remapping is necessary. > > Correct. > >> >> 2. For each vmlinux type in our list of relocations, check its >> compatibility with the associated vmlinux type. This is >> somewhat akin to the CO-RE compatibility checks. Exact rules > > Not really. CO-RE compatiblity is explicitly very permissive, while > here we want to make sure that types are actually memory > layout-compatible. > >> would need to be ironed out, but a somewhat loose approach >> would be ideal such that a few minor changes in a struct >> somewhere do not totally invalidate module BTF. Unlike CO-RE >> though, field offset changes are _not_ good since they imply the >> module has an incorrect view of the structure and might >> start using fields incorrectly. > > I think vmlinux type should have at least all the members that module > expects, at the same offset, with the same size. Maybe we should allow > vmlinux type to get some types at the end, not sure. How hard a > requirement it is to accommodate non-exact type matches between > vmlinux and kernel module's types? > The main need is to support resilience in the face of small structure changes such that the compiled module will still work. When backporting fixes to a stable-based kernel - where a version of say 5.15 stable is supported for a while and so accumulates stable fixes - often the approach used is to use holes in structures for new fields, or if the structure is not embedded in any module-specific structures, add fields at the end. All existing field offsets should match. In taking that approach, the aim is to make sure data accesses in the module are still valid - memory layout compatibility is the goal. >> >> Note that this is a bit easier than BTF deduplication, because >> the deduplication process that happened at module encoding time >> has already done the dependency checking for us; we just need >> to do a type-by-type, 1-to-1 comparison between our relocation >> types and current vmlinux types. >> >> 3. If all types are consistent, BTF is loaded and we remap the >> module's vmlinux BTF id references to the corresponding >> vmlinux BTF ids of the current vmlinux. > > Note that we might need to do something special for anonymous types > (modifiers, anon enums and structs/unions). Otherwise it's not clear > how to even map them between vmlinux BTF and module BTF. > Good point, we'd probably need to represent some sort of parent-child relationship to handle cases like this. >> >> I _think_ this gets us what we want; more resilient module BTF, >> but with safety checks to ensure compatible representations. >> There were some suggestions of using a hashing method, but I think >> such a method presupposes we want exact type matches, which I suspect >> would be unlikely to be useful in practice as with most stable-based >> distros, small changes in types can be made due to fixes etc. > > What are "small changes" and how are they automatically determined and > validated? > See above, field additions in data structure holes or appended to structs for the most part. Once I have something rough working I'll see how it performs in practice and report back. Thanks! Alan >> >> There were also a suggestion of doing a full dedup, but I think the >> consensus in the room (which I agree with) is that would be hard >> to do in-kernel. So the above approach is a compropmise I think; >> it gets actual dedup at BTF creation time to create the list of >> references and dependents, and we later check them one-by-one on module >> load for compatibility. >> >> Anyway I just wanted to try and capture the feedback received, and >> lay out a possible direction. Any further thoughts or suggestions >> would be much appreciated. Thanks! >> >> Alan >> >> [1] https://lpc.events/event/17/contributions/1576/