[Crash-utility] Extensions ABI compatibility

[lijiang]

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
>