Hi, Mathias Thank you for discussing this topic and describing its backgrounds. 在 2020年08月19日 23:45, Mathias Krause 写道: > Hi list, > > during development of the PaX module patch[1], Lianbo pointed out an > issue about adding new members to struct offset_table. They should be > added to the end to ensure binary compatibility with already compiled > extensions, as these won't be aware of the new members and, even worse, > might be using wrong offsets within the offset_table if we didn't. As > this seems to be rather fragile, here's a proposal to make it less so. > > To not blindly make already compiled extensions misbehave by using the > wrong offsets when new members get added in the middle of shared types, > we need to have some validation step, preferably during extension > registration to catch these. > Sounds good to me. > If we want to keep binary compatibility, we need to add members to the > end. That's a rule maintainers can enforce but as we all make mistakes, Recently, we are considering putting the crash guidelines on the wiki in order to help developers avoid making mistakes. > code can help us here. Changing a shared data structure has, at least, > implications on its size. So we can use this as a base to build checks > on. If the size of a data structure used during compilation of an > extension differs from the size of that same data structure as seen > within 'crash', we have an ABI issue. The response should be, at least, > a warning to inform the user to recompile its extension to ensure ABI > compatibility. Another option would be to decline loading the extension > but that might be inconvenient for users, if we indeed made sure to add > members only at the end of the involved data structures. > > These checks can be implemented "transparently" as sketched in the > attached patch. It changes register_extension() to be a macro that > stores the sizes of critical types in a data structure that gets passed > to _register_extension() which is implemented in 'crash' and will do the > size checks against its version of the involved types. > > The patch also takes care about backward compatibility by providing a > stub register_extension() function that passes a NULL structure, as all > current extension modules refer to register_extension() instead of the > newly introduced _register_extension(). > > Open TODOs: > 1/ Compile a list of all types we want to protect and add checks for > these. > 2/ Put these types into a 'shared.h' header file and use that > exclusively within extensions -- maybe even enforce it. > > Do we want to follow that approach or look for alternatives? > Let's wait for more comments or suggestions. Thanks. Lianbo > Regards, > Mathias > > [1]: https://www.redhat.com/archives/crash-utility/2020-August/msg00024.html > -- Crash-utility mailing list Crash-utility@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/crash-utility
