From d7c082bc0d269dcc901da6f5ef199051fa9eab71 Mon Sep 17 00:00:00 2001 From: Balint Reczey Date: Tue, 19 Dec 2023 22:02:04 +0100 Subject: [PATCH] Revert "Refactor plugin registration and add ABI/license check" This reverts commit 90b16b40921b737aadf9186685d866fd80e37ee6. --- CMakeLists.txt | 16 ---- doc/plugins.example/hello.c | 27 ++++--- plugins/epan/dfilter/ipaddr/ipaddr.c | 25 +++--- tools/make-plugin-reg.py | 57 +++++-------- ws_version.h.in | 5 -- wsutil/plugins.c | 116 +++++++++++---------------- wsutil/plugins.h | 58 ++------------ 7 files changed, 101 insertions(+), 203 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 665d4f15e3..dd9b78c395 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -59,22 +59,6 @@ set(PROJECT_MAJOR_VERSION 4) set(PROJECT_MINOR_VERSION 3) set(PROJECT_PATCH_VERSION 0) set(PROJECT_BUILD_VERSION 0) -# ABI version for plugins. It must be incremented by one for -# every odd minor release number. -# start at 1 (arbitrary). -# release 4.3.X (unstable) = ABI version 1 (start) -# release 4.4.X (stable) = ABI version 1 (stable release, freeze ABI 1) -# release 4.5.X (unstable) = ABI version 2 (increment for new unstable release, ABI break) -# release 4.6.X (stable) = ABI version 2 (stable release, freeze ABI 2) -# release 4.7.X (unstable) = ABI version 3 (increment for new unstable release, ABI break) -# release 5.0.X (stable) = ABI version 3 (stable release, freeze ABI 3) -# etc. -set(PROJECT_ABI_VERSION_EPAN 1) -set(PROJECT_ABI_VERSION_WIRETAP 1) -# Codecs API/ABI is much more narrow and stable than the other two so this -# may not need to be incremented every X.Y release. -set(PROJECT_ABI_VERSION_CODEC 1) - set(PROJECT_VERSION_EXTENSION "") if(DEFINED ENV{WIRESHARK_VERSION_EXTRA}) diff --git a/doc/plugins.example/hello.c b/doc/plugins.example/hello.c index 19178bfeab..5a9ccd8169 100644 --- a/doc/plugins.example/hello.c +++ b/doc/plugins.example/hello.c @@ -17,13 +17,20 @@ #define VERSION "0.0.0" #endif +WS_DLL_PUBLIC_DEF const char plugin_version[] = VERSION; +WS_DLL_PUBLIC_DEF const int plugin_want_major = WIRESHARK_VERSION_MAJOR; +WS_DLL_PUBLIC_DEF const int plugin_want_minor = WIRESHARK_VERSION_MINOR; + +WS_DLL_PUBLIC void plugin_register(void); +WS_DLL_PUBLIC uint32_t plugin_describe(void); + static int proto_hello; static dissector_handle_t handle_hello; static int dissect_hello(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tree *tree, void *data _U_) { - proto_tree_add_protocol_format(tree, proto_hello, tvb, 0, -1, "This is Hello version %s, a Wireshark postdissector plugin prototype", VERSION); + proto_tree_add_protocol_format(tree, proto_hello, tvb, 0, -1, "This is Hello version %s, a Wireshark postdissector plugin prototype", plugin_version); return tvb_captured_length(tvb); } @@ -41,7 +48,7 @@ proto_reg_handoff_hello(void) /* empty */ } -static void +void plugin_register(void) { static proto_plugin plug; @@ -51,14 +58,8 @@ plugin_register(void) proto_register_plugin(&plug); } -static struct ws_module module = { - .license = WS_PLUGIN_IS_GPLv2_OR_LATER, - .flags = WS_PLUGIN_DESC_DISSECTOR, - .version = VERSION, - .spdx_id = "GPL-2.0-or-later", - .home_url = "Your-URL-here", - .blurb = "Hello world for Wireshark plugin development", - .register_cb = &plugin_register, -}; - -WIRESHARK_PLUGIN_REGISTER_EPAN(&module, 0) +uint32_t +plugin_describe(void) +{ + return WS_PLUGIN_DESC_DISSECTOR; +} diff --git a/plugins/epan/dfilter/ipaddr/ipaddr.c b/plugins/epan/dfilter/ipaddr/ipaddr.c index 857eb1f3f7..f6f7ded5e7 100644 --- a/plugins/epan/dfilter/ipaddr/ipaddr.c +++ b/plugins/epan/dfilter/ipaddr/ipaddr.c @@ -19,6 +19,13 @@ #define PLUGIN_VERSION "0.0.0" #endif +WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION; +WS_DLL_PUBLIC_DEF const int plugin_want_major = WIRESHARK_VERSION_MAJOR; +WS_DLL_PUBLIC_DEF const int plugin_want_minor = WIRESHARK_VERSION_MINOR; + +WS_DLL_PUBLIC void plugin_register(void); +WS_DLL_PUBLIC uint32_t plugin_describe(void); + typedef bool (*ip_is_func_t)(fvalue_t *); static const struct ws_iana_ip_special_block * @@ -338,7 +345,7 @@ cleanup(void) df_func_deregister(&func_ip_is_ula); } -static void +void plugin_register(void) { static dfilter_plugin plug; @@ -348,14 +355,8 @@ plugin_register(void) dfilter_plugins_register(&plug); } -static struct ws_module module = { - .license = WS_PLUGIN_IS_GPLv2_OR_LATER, - .flags = WS_PLUGIN_DESC_DFILTER, - .version = PLUGIN_VERSION, - .spdx_id = WS_PLUGIN_SPDX_GPLv2, - .home_url = WS_PLUGIN_GITLAB_URL, - .blurb = "Display filter functions to test IP addresses", - .register_cb = &plugin_register, -}; - -WIRESHARK_PLUGIN_REGISTER_EPAN(&module, 0) +uint32_t +plugin_describe(void) +{ + return WS_PLUGIN_DESC_DFILTER; +} diff --git a/tools/make-plugin-reg.py b/tools/make-plugin-reg.py index 34df09a847..cd2de2f21d 100755 --- a/tools/make-plugin-reg.py +++ b/tools/make-plugin-reg.py @@ -19,11 +19,6 @@ srcdir = sys.argv[1] # registertype = sys.argv[2] # -# The third argument is the plugin short description -# -#plugin_blurb = sys.argv[3] -plugin_blurb = "FIXME" -# # All subsequent arguments are the files to scan. # files = sys.argv[3:] @@ -145,11 +140,29 @@ for symbol in regs['codec_register']: for symbol in regs['register_tap_listener']: reg_code += "void register_tap_listener_%s(void);\n" % (symbol) +DESCRIPTION_FLAG = { + 'plugin': 'WS_PLUGIN_DESC_DISSECTOR', + 'plugin_wtap': 'WS_PLUGIN_DESC_FILE_TYPE', + 'plugin_codec': 'WS_PLUGIN_DESC_CODEC', + 'plugin_tap': 'WS_PLUGIN_DESC_TAP_LISTENER' +} + reg_code += """ -static void -plugin_register(void) +WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION; +WS_DLL_PUBLIC_DEF const int plugin_want_major = VERSION_MAJOR; +WS_DLL_PUBLIC_DEF const int plugin_want_minor = VERSION_MINOR; + +WS_DLL_PUBLIC void plugin_register(void); +WS_DLL_PUBLIC uint32_t plugin_describe(void); + +uint32_t plugin_describe(void) { -""" + return %s; +} + +void plugin_register(void) +{ +""" % DESCRIPTION_FLAG[registertype] if registertype == "plugin": for symbol in regs['proto_reg']: @@ -178,34 +191,6 @@ if registertype == "plugin_tap": reg_code += "}\n" -DESCRIPTION_FLAG = { - 'plugin': 'WS_PLUGIN_DESC_DISSECTOR', - 'plugin_wtap': 'WS_PLUGIN_DESC_FILE_TYPE', - 'plugin_codec': 'WS_PLUGIN_DESC_CODEC', - 'plugin_tap': 'WS_PLUGIN_DESC_TAP_LISTENER' -} - -PLUGIN_REGISTER = { - 'plugin': 'WIRESHARK_PLUGIN_REGISTER_EPAN', - 'plugin_wtap': 'WIRESHARK_PLUGIN_REGISTER_WIRETAP', - 'plugin_codec': 'WIRESHARK_PLUGIN_REGISTER_CODEC', - 'plugin_tap': 'WIRESHARK_PLUGIN_REGISTER_EPAN' -} - -reg_code += """ -static struct ws_module module = { - .license = WS_PLUGIN_IS_GPLv2_OR_LATER, - .flags = %s, - .version = PLUGIN_VERSION, - .spdx_id = WS_PLUGIN_SPDX_GPLv2, - .home_url = WS_PLUGIN_GITLAB_URL, - .blurb = "%s", - .register_cb = &plugin_register, -}; - -%s(&module, 0) -""" % (DESCRIPTION_FLAG[registertype], plugin_blurb, PLUGIN_REGISTER[registertype]) - try: fh = open(final_filename, 'w') fh.write(reg_code) diff --git a/ws_version.h.in b/ws_version.h.in index 3e6548d83c..f89bfb80e2 100644 --- a/ws_version.h.in +++ b/ws_version.h.in @@ -7,9 +7,4 @@ #define WIRESHARK_VERSION_MINOR @PROJECT_MINOR_VERSION@ #define WIRESHARK_VERSION_MICRO @PROJECT_PATCH_VERSION@ -/* ABI version for plugin compatibility. */ -#define WIRESHARK_ABI_VERSION_EPAN @PROJECT_ABI_VERSION_EPAN@ -#define WIRESHARK_ABI_VERSION_WIRETAP @PROJECT_ABI_VERSION_WIRETAP@ -#define WIRESHARK_ABI_VERSION_CODEC @PROJECT_ABI_VERSION_CODEC@ - #endif /* __WS_VERSION_H__ */ diff --git a/wsutil/plugins.c b/wsutil/plugins.c index d8969d4608..69a4710a08 100644 --- a/wsutil/plugins.c +++ b/wsutil/plugins.c @@ -29,7 +29,8 @@ typedef struct _plugin { GModule *handle; /* handle returned by g_module_open */ char *name; /* plugin name */ - struct ws_module *module; + const char *version; /* plugin version */ + uint32_t flags; /* plugin flags */ } plugin; #define TYPE_DIR_EPAN "epan" @@ -56,22 +57,6 @@ type_to_dir(plugin_type_e type) ws_assert_not_reached(); } -static inline const char * -type_to_name(plugin_type_e type) -{ - switch (type) { - case WS_PLUGIN_EPAN: - return "epan"; - case WS_PLUGIN_WIRETAP: - return "wiretap"; - case WS_PLUGIN_CODEC: - return "codec"; - default: - return "unknown"; - } - ws_assert_not_reached(); -} - static inline const char * flags_to_str(uint32_t flags) { @@ -109,20 +94,26 @@ compare_plugins(gconstpointer a, gconstpointer b) } static bool -pass_plugin_compatibility(const char *name, plugin_type_e type, - int abi_version, int min_api_level _U_, - struct ws_module *module) +pass_plugin_version_compatibility(GModule *handle, const char *name) { - if (abi_version != plugins_abi_version(type)) { - report_failure("The plugin '%s' has incompatible ABI, have version %d, expected %d", - name, abi_version, plugins_abi_version(type)); + void * symb; + int major, minor; + + if(!g_module_symbol(handle, "plugin_want_major", &symb)) { + report_failure("The plugin '%s' has no \"plugin_want_major\" symbol", name); return false; } + major = *(int *)symb; - if (module->license != WS_PLUGIN_IS_GPLv2_OR_LATER && - module->license != WS_PLUGIN_IS_GPLv2_COMPATIBLE) { - report_failure("The plugin '%s' is not GPLv2 compatible (invalid license 0x%x, with SPDX ID \"%s\")", - name, module->license, module->spdx_id); + if(!g_module_symbol(handle, "plugin_want_minor", &symb)) { + report_failure("The plugin '%s' has no \"plugin_want_minor\" symbol", name); + return false; + } + minor = *(int *)symb; + + if (major != VERSION_MAJOR || minor != VERSION_MINOR) { + report_failure("The plugin '%s' was compiled for Wireshark version %d.%d", + name, major, minor); return false; } @@ -144,12 +135,10 @@ scan_plugins_dir(GHashTable *plugins_module, const char *dirpath, plugin_type_e char *plugin_folder; char *plugin_file; /* current file full path */ GModule *handle; /* handle returned by g_module_open */ - void *symbol; + void * symbol; + const char *plug_version; + uint32_t flags; plugin *new_plug; - plugin_type_e have_type; - int abi_version; - int min_api_level; - struct ws_module *module; if (append_type) plugin_folder = g_build_filename(dirpath, type_to_dir(type), (char *)NULL); @@ -189,41 +178,45 @@ scan_plugins_dir(GHashTable *plugins_module, const char *dirpath, plugin_type_e continue; } + if (!g_module_symbol(handle, "plugin_version", &symbol)) + { + report_failure("The plugin '%s' has no \"plugin_version\" symbol", name); + g_module_close(handle); + g_free(plugin_file); + continue; + } + plug_version = (const char *)symbol; + + if (!pass_plugin_version_compatibility(handle, name)) { + g_module_close(handle); + g_free(plugin_file); + continue; + } + /* Search for the entry point for the plugin registration function */ - if (!g_module_symbol(handle, "wireshark_load_module", &symbol)) { - report_failure("The plugin '%s' has no \"wireshark_load_module\" symbol", name); + if (!g_module_symbol(handle, "plugin_register", &symbol)) { + report_failure("The plugin '%s' has no \"plugin_register\" symbol", name); g_module_close(handle); g_free(plugin_file); continue; } DIAG_OFF_PEDANTIC - /* Found it, load module. */ - have_type = ((ws_load_module_func)symbol)(&abi_version, &min_api_level, &module); + /* Found it, call the plugin registration function. */ + ((plugin_register_func)symbol)(); DIAG_ON_PEDANTIC - if (have_type != type) { - // Should not happen. Our filesystem hierarchy uses plugin type. - report_failure("The plugin '%s' has invalid type, expected %s, have %s", - name, type_to_name(type), type_to_name(have_type)); - g_module_close(handle); - g_free(plugin_file); - continue; - } - - if (!pass_plugin_compatibility(name, type, abi_version, min_api_level, module)) { - g_module_close(handle); - g_free(plugin_file); - continue; - } - - /* Call the plugin registration function. */ - module->register_cb(); + /* Search for the (optional) description flag registration function */ + if (g_module_symbol(handle, "plugin_describe", &symbol)) + flags = ((plugin_describe_func)symbol)(); + else + flags = 0; new_plug = g_new(plugin, 1); new_plug->handle = handle; new_plug->name = g_strdup(name); - new_plug->module = module; + new_plug->version = plug_version; + new_plug->flags = flags; /* Add it to the list of plugins. */ g_hash_table_replace(plugins_module, new_plug->name, new_plug); @@ -285,8 +278,7 @@ plugins_get_descriptions(plugin_description_callback callback, void *callback_da for (unsigned i = 0; i < plugins_array->len; i++) { plugin *plug = (plugin *)plugins_array->pdata[i]; - callback(plug->name, plug->module->version, plug->module->flags, - g_module_name(plug->handle), callback_data); + callback(plug->name, plug->version, plug->flags, g_module_name(plug->handle), callback_data); } g_ptr_array_free(plugins_array, true); @@ -333,18 +325,6 @@ plugins_supported(void) return g_module_supported(); } -int -plugins_abi_version(plugin_type_e type) -{ - switch (type) { - case WS_PLUGIN_EPAN: return WIRESHARK_ABI_VERSION_EPAN; - case WS_PLUGIN_WIRETAP: return WIRESHARK_ABI_VERSION_WIRETAP; - case WS_PLUGIN_CODEC: return WIRESHARK_ABI_VERSION_CODEC; - default: return -1; - } - ws_assert_not_reached(); -} - /* * Editor modelines * diff --git a/wsutil/plugins.h b/wsutil/plugins.h index fa4768f610..113f1f687d 100644 --- a/wsutil/plugins.h +++ b/wsutil/plugins.h @@ -17,25 +17,17 @@ extern "C" { #endif /* __cplusplus */ +typedef void (*plugin_register_func)(void); +typedef uint32_t (*plugin_describe_func)(void); + +typedef void plugins_t; + typedef enum { WS_PLUGIN_EPAN, WS_PLUGIN_WIRETAP, WS_PLUGIN_CODEC } plugin_type_e; -typedef enum { - /* Plug-in license is GPLv2-or-later */ - WS_PLUGIN_IS_GPLv2_OR_LATER = 0x2222, /* Ok */ - /* Plug-in license is compatible with the GPL version 2, according to the FSF. */ - /* https://www.gnu.org/licenses/gpl-faq.html#WhatDoesCompatMean */ - WS_PLUGIN_IS_GPLv2_COMPATIBLE = 0x2002, /* Ok */ - /* Plug-in license is none of the above */ - WS_PLUGIN_IS_GPLv2_INCOMPATIBLE = 0, /* Not allowed, will refuse to load.*/ -} plugin_license_e; - -#define WS_PLUGIN_SPDX_GPLv2 "GPL-2.0-or-later" -#define WS_PLUGIN_GITLAB_URL "https://gitlab.com/wireshark/wireshark" - #define WS_PLUGIN_DESC_DISSECTOR (1UL << 0) #define WS_PLUGIN_DESC_FILE_TYPE (1UL << 1) #define WS_PLUGIN_DESC_CODEC (1UL << 2) @@ -43,23 +35,6 @@ typedef enum { #define WS_PLUGIN_DESC_TAP_LISTENER (1UL << 4) #define WS_PLUGIN_DESC_DFILTER (1UL << 5) -typedef void plugins_t; - -typedef void (*module_register_func)(void); - -struct ws_module { - plugin_license_e license; - int min_api_level; /* unused */ - uint32_t flags; - const char *version; - const char *spdx_id; - const char *home_url; - const char *blurb; - module_register_func register_cb; -}; - -typedef plugin_type_e (*ws_load_module_func)(int *, int *, struct ws_module **); - WS_DLL_PUBLIC plugins_t *plugins_init(plugin_type_e type); typedef void (*plugin_description_callback)(const char *name, const char *version, @@ -76,29 +51,6 @@ WS_DLL_PUBLIC void plugins_cleanup(plugins_t *plugins); WS_DLL_PUBLIC bool plugins_supported(void); -WS_DLL_PUBLIC -int plugins_abi_version(plugin_type_e type); - -#define WIRESHARK_PLUGIN_REGISTER(type, ptr_, api_level_) \ - WS_DLL_PUBLIC plugin_type_e \ - wireshark_load_module(int *abi_version, int *min_api_level, \ - struct ws_module **plug_ptr) \ - { \ - *abi_version = WIRESHARK_ABI_VERSION_ ## type; \ - *min_api_level = api_level_; \ - *plug_ptr = ptr_; \ - return WS_PLUGIN_ ## type; \ - } - -#define WIRESHARK_PLUGIN_REGISTER_EPAN(ptr, level) \ - WIRESHARK_PLUGIN_REGISTER(EPAN, ptr, level) - -#define WIRESHARK_PLUGIN_REGISTER_WIRETAP(ptr, level) \ - WIRESHARK_PLUGIN_REGISTER(WIRETAP, ptr, level) - -#define WIRESHARK_PLUGIN_REGISTER_CODEC(ptr, level) \ - WIRESHARK_PLUGIN_REGISTER(CODEC, ptr, level) - #ifdef __cplusplus } #endif /* __cplusplus */