Problem
The current plugin loader has a forward compatibility issue, and could cause segfault scenarios when using older plugins with newer versions of OpenHPI.
The crux of the problem has to do with the way in which symbols are pulled from plugin objects. In OpenHPI 2.0 there is a single exported symbol get_interface which is pulled via the following code:
src/plugin.c: 312
get_interface = lt_dlsym(plugin->dl_handle, "get_interface");
if (!get_interface) {
dbg("Can not get 'get_interface' symbol, is it a plugin?!");
goto err1;
}
err = get_interface(&plugin->abi, UUID_OH_ABI_V2);
if (err < 0 || !plugin->abi || !plugin->abi->open) {
dbg("Can not get ABI V1");
goto err1;
}
While the call which retrieves get_interface is relatively safe (as the non existance of get_interface just produces a NULL, which is caught) the fact that get_interface fills out the plugin->abi is not. If a plugin was compiled with an old version of the oh_handler.h header one of 2 things will happen.
the plugin can not load at all as UUID_OH_ABI_V2 doesn't version match, while this provides safety it means that for ever plugin api change ever plugin must be completely rebuilt, even if that change was the add of a new function.
- the plugin will load (because someone forgot to update UUID_OH_ABI_V2, which happened a lot in the 1.9 devel stream). The size of the abi pointer will be different between what the plugin thinks it is and what infrastructure thinks it is. So calls to last element of the new api will run off the end of the abi struct and cause a segfault, or something even more serious.
The current structure also violates our long standing rule of no void poiters crossing the barrier between plugins and infrastructure and being directly accessed from both sides. Though it is simple to do, it isn't safe.
Solutions
The Libtool DL Approach
The first possible solution is to lean on the dynamic loader a lot more than we do today. Why should we only use it to extract 1 symbol when we can extract all 50. That would provide use with a very accurate map of what functions are there, and what are not. It also means that plugins would only have to implement the functions they supported, and not provide dummies for the rest.
This would be that plugin->api would stay the same, but the filling of the structure would happen run time in oh_plugin.c, not compile time in each of the individual plugins.
Advantages
- no more versioning of the interface, the functions themselves are the version control
- easy to roll forward with plugin functions, especially for experimenting
Disadvantages
- requires all plugins to port to the new API once it is defined
- not sure how static loading would work with this, however as only Force ever had an interest in static loading (and they are no longer project members), I'm not sure this is a big deal.
The oh_check_version Approach
Another way this could be done is to check the version of the api not on load but on every function call. In theory, if functions were only added to the end of the structure, then it would be clear that we could check the version api first before trying to access a pointer. If oh_check_version said we were safe, we could continue.
Advantages
- no need to change plugins from current api
- static works like it does today
Disadvantages
- still not completely safe, reorder of the structure for whatever reason would kill us
- still relying on void pointer between plugin and infrastructure, which can cause problems
- a lot of complexity would be put into the runtime checks
