Remove support for external plugins

Greetings one and all,

In this series, we prune support for external plugins and cleanup the associated code. The inspiration here is multiple-fold:
- the plugins are under linked - generally a bad idea
- the plugins use undefined, unscoped, unversioned internal API
- the main daemons expose their internal API increasing their size

So this series:
- bluetoothd: removes the "dummy" external plugin
- bluetoothd: converts the sixaxis plugin to builtin
- bluetoothd, obexd: removes support for external plugins
- bluetoothd, obexd: cleans the exported symbols by the daemons
- android: (bonus) export only the required HMI plugin entrypoint

Please note:
- expect minor conflicts wrt my earlier patches - happy to respin
- there _might_ be third-party external plugins, that my distro search have missed

This concludes the paper cut patches. Expect some more around config file handling in the upcoming days :-)

Thanks
Emil

---
Emil Velikov (8):
obexd: remove support for external plugins
build: don't export internal obexd API
plugins: remove external-dummy
plugins: convert external sixaxis plugin to builtin
bluetoothd: remove support for external plugins
bluetoothd: remove debug support for external plugins
bluetoothd: don't export internal API
android: export only (android) entrypoint from the modules

 Makefile.am              | 21 +-------------
 Makefile.obexd           |  8 ++----
 Makefile.plugins         |  8 ++----
 android/Makefile.am      |  3 ++
 android/hal-audio.c      |  1 +
 android/hal-bluetooth.c  |  1 +
 android/hal-sco.c        |  1 +
 obexd/src/obexd.h        |  2 +-
 obexd/src/plugin.c       | 73 ++++-------------------------------------------
 obexd/src/plugin.h       |  9 ------
 plugins/external-dummy.c | 28 ------------------
 src/bluetooth.ver        | 20 -------------
 src/btd.h                |  2 +-
 src/log.c                | 10 ++-----
 src/log.h                |  3 +-
 src/plugin.c             | 74 ++++--------------------------------------------
 src/plugin.h             | 16 -----------
 17 files changed, 30 insertions(+), 250 deletions(-)

---
base-commit: 770ad5614e7e8074133e6f563495ce4822f63fe4
change-id: 20240116-rm-ext-plugins-ba0b852a492b

Best regards,

Emil Velikov <[email protected]>

From: Emil Velikov <[email protected]>

A while ago all the plugins were converted to built-in, although the external machinery remained - remove it.

In practise, this means we no longer need to export obexd internal API (fix coming in later patch). AFACIT supporting third-party plugins was never a supported use-case.

Glancing around - no Linux distros seem to ship plugins, these days.

---
 Makefile.obexd     |  6 +----
 obexd/src/obexd.h  |  2 +-
 obexd/src/plugin.c | 73 +++++-------------------------------------------------
 obexd/src/plugin.h |  9 -------
 4 files changed, 8 insertions(+), 82 deletions(-)

index 5d1a4ff65..2774f3aec 100644
@@ -11,8 +11,6 @@ EXTRA_DIST += obexd/src/obex.service.in obexd/src/org.bluez.obex.service
 
 if OBEX
 
-obex_plugindir = $(libdir)/obex/plugins
-
 obexd_builtin_modules =
 obexd_builtin_sources =
 obexd_builtin_nodist =

@@ -89,9 +87,7 @@ obexd_src_obexd_LDADD = lib/libbluetooth-internal.la \
 obexd_src_obexd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic
 
 obexd_src_obexd_CPPFLAGS = $(AM_CPPFLAGS) $(GLIB_CFLAGS) $(DBUS_CFLAGS) \
-				$(ICAL_CFLAGS) -DOBEX_PLUGIN_BUILTIN \
-				-DPLUGINDIR=\""$(obex_plugindir)"\" \
-				-D_FILE_OFFSET_BITS=64 \
+				$(ICAL_CFLAGS) -D_FILE_OFFSET_BITS=64 \
 				-I$(builddir)/lib -I$(builddir)/obexd/src
 
 obexd_src_obexd_CFLAGS = $(AM_CFLAGS) -fPIC

index fe312a65b..af5265da5 100644
@@ -18,7 +18,7 @@
 #define OBEX_MAS	(1 << 8)
 #define OBEX_MNS	(1 << 9)
 
-gboolean plugin_init(const char *pattern, const char *exclude);
+void plugin_init(const char *pattern, const char *exclude);
 void plugin_cleanup(void);
 
 gboolean manager_init(void);

index 703878460..2df66c79b 100644
@@ -14,16 +14,7 @@ struct obex_plugin_desc {
 	void (*exit) (void);
 };
 
-#ifdef OBEX_PLUGIN_BUILTIN
 #define OBEX_PLUGIN_DEFINE(name, init, exit) \
 		struct obex_plugin_desc __obex_builtin_ ## name = { \
 			#name, init, exit \
 		};
-#else
-#define OBEX_PLUGIN_DEFINE(name,init,exit) \
-		extern struct obex_plugin_desc obex_plugin_desc \
-				__attribute__ ((visibility("default"))); \
-		struct obex_plugin_desc obex_plugin_desc = { \
-			#name, init, exit \
-		};
-#endif

2.43.0

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=817230

---Test result---

Test Summary:

CheckPatch                    PASS      3.87 seconds
GitLint                       PASS      2.60 seconds
BuildEll                      PASS      23.85 seconds
BluezMake                     PASS      702.42 seconds
MakeCheck                     PASS      11.69 seconds
MakeDistcheck                 PASS      159.11 seconds
CheckValgrind                 PASS      221.12 seconds
CheckSmatch                   PASS      326.34 seconds
bluezmakeextell               PASS      106.35 seconds
IncrementalBuild              PASS      5304.46 seconds
ScanBuild                     PASS      928.69 seconds

---
Regards,
Linux Bluetooth

From: Emil Velikov <[email protected]>

Unlike bluetoothd, obexd does not support external plugins. As such it should not export any functions. If that ever change and plugins do emerge, the symbols should be controlled via a version script.

---
 Makefile.obexd | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 2774f3aec..4e473d50b 100644
@@ -84,7 +84,7 @@ obexd_src_obexd_LDADD = lib/libbluetooth-internal.la \
 			$(ICAL_LIBS) $(DBUS_LIBS) $(LIBEBOOK_LIBS) \
 			$(LIBEDATASERVER_LIBS) $(GLIB_LIBS) -ldl
 
-obexd_src_obexd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic
+obexd_src_obexd_LDFLAGS = $(AM_LDFLAGS)
 
 obexd_src_obexd_CPPFLAGS = $(AM_CPPFLAGS) $(GLIB_CFLAGS) $(DBUS_CFLAGS) \
 				$(ICAL_CFLAGS) -D_FILE_OFFSET_BITS=64 \

2.43.0

From: Emil Velikov <[email protected]>

The external plugins infra is going away - remove this dummy plugin.

---
 Makefile.am              |  8 --------
 plugins/external-dummy.c | 28 ----------------------------
 2 files changed, 36 deletions(-)

index e738eb3a5..ea51b25cc 100644
@@ -293,14 +293,6 @@ builtin_ldadd =
 
 include Makefile.plugins
 
-if MAINTAINER_MODE
-plugin_LTLIBRARIES += plugins/external-dummy.la
-plugins_external_dummy_la_SOURCES = plugins/external-dummy.c
-plugins_external_dummy_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
-				    -no-undefined
-plugins_external_dummy_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
-endif
-
 pkglibexec_PROGRAMS += src/bluetoothd
 
 src_bluetoothd_SOURCES = $(builtin_sources) \

deleted file mode 100644
index 1c209e8b7..000000000
@@ -1,28 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- *
- *  BlueZ - Bluetooth protocol stack for Linux
- *
- */
-
-#ifdef HAVE_CONFIG_H
-#include <config.h>
-#endif
-
-#include "src/plugin.h"
-#include "src/log.h"
-
-static int dummy_init(void)
-{
-	DBG("");
-
-	return 0;
-}
-
-static void dummy_exit(void)
-{
-	DBG("");
-}
-
-BLUETOOTH_PLUGIN_DEFINE(external_dummy, VERSION,
-		BLUETOOTH_PLUGIN_PRIORITY_LOW, dummy_init, dummy_exit)

2.43.0

From: Emil Velikov <[email protected]>

Sixaxis plugin is the only external one. It's a tiny 20K binary that distros ship a separate package for.

Make it a builtin, which allows distros to drop the separate package, it also enables us to remove support for external modules - both in terms of extra code and hide the internal bluetoothd API.

This means that libudev.so is pulled in, which is fine since its ABI has been stable for over a decade.

---
 Makefile.plugins | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

index 5880ed0df..7cf66fd59 100644
@@ -110,11 +110,9 @@ builtin_modules += battery
 builtin_sources += profiles/battery/battery.c
 
 if SIXAXIS
-plugin_LTLIBRARIES += plugins/sixaxis.la
-plugins_sixaxis_la_SOURCES = plugins/sixaxis.c
-plugins_sixaxis_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version
-plugins_sixaxis_la_LIBADD = $(UDEV_LIBS)
-plugins_sixaxis_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
+builtin_modules += sixaxis
+builtin_sources += plugins/sixaxis.c
+builtin_ldadd += $(UDEV_LIBS)
 endif
 
 if BAP

2.43.0

From: Emil Velikov <[email protected]>

With the final one converted to a builtin, we can drop the now dead code. As follow-up this will allow us to stop exposing the internal API of bluetoothd, reducing its size.

---
 Makefile.am  | 10 +--------
 src/btd.h    |  2 +-
 src/plugin.c | 72 +++++-------------------------------------------------------
 src/plugin.h | 14 ------------
 4 files changed, 8 insertions(+), 90 deletions(-)

index ea51b25cc..4db3a2953 100644
@@ -51,12 +51,6 @@ EXTRA_DIST += src/bluetooth.service.in src/org.bluez.service
 
 plugindir = $(libdir)/bluetooth/plugins
 
-if MAINTAINER_MODE
-build_plugindir = $(abs_top_srcdir)/plugins/.libs
-else
-build_plugindir = $(plugindir)
-endif
-
 if MANPAGES
 man_MANS =
 endif

@@ -337,9 +331,7 @@ src_bluetoothd_DEPENDENCIES = lib/libbluetooth-internal.la \
 				src/libshared-glib.la \
 				src/bluetooth.service
 
-src_bluetoothd_CPPFLAGS = $(AM_CPPFLAGS) -DBLUETOOTH_PLUGIN_BUILTIN \
-					-DPLUGINDIR=\""$(build_plugindir)"\" \
-					$(BACKTRACE_CFLAGS) $(builtin_cppflags)
+src_bluetoothd_CPPFLAGS = $(AM_CPPFLAGS) $(BACKTRACE_CFLAGS) $(builtin_cppflags)
 src_bluetoothd_SHORTNAME = bluetoothd
 
 builtin_files = src/builtin.h $(builtin_nodist)

index b7e7ebd61..7166e2168 100644
@@ -155,7 +155,7 @@ struct btd_opts {
 
 extern struct btd_opts btd_opts;
 
-gboolean plugin_init(const char *enable, const char *disable);
+void plugin_init(const char *enable, const char *disable);
 void plugin_cleanup(void);
 
 void rfkill_init(void);

index a5f92a557..7ff55e796 100644
@@ -21,21 +21,7 @@ struct bluetooth_plugin_desc {
 	void *debug_stop;
 };
 
-#ifdef BLUETOOTH_PLUGIN_BUILTIN
 #define BLUETOOTH_PLUGIN_DEFINE(name, version, priority, init, exit) \
 		struct bluetooth_plugin_desc __bluetooth_builtin_ ## name = { \
 			#name, version, priority, init, exit \
 		};
-#else
-#define BLUETOOTH_PLUGIN_DEFINE(name, version, priority, init, exit) \
-		extern struct btd_debug_desc __start___debug[] \
-				__attribute__ ((weak, visibility("hidden"))); \
-		extern struct btd_debug_desc __stop___debug[] \
-				__attribute__ ((weak, visibility("hidden"))); \
-		extern struct bluetooth_plugin_desc bluetooth_plugin_desc \
-				__attribute__ ((visibility("default"))); \
-		struct bluetooth_plugin_desc bluetooth_plugin_desc = { \
-			#name, version, priority, init, exit, \
-			__start___debug, __stop___debug \
-		};
-#endif

2.43.0

From: Emil Velikov <[email protected]>

External plugins are gone, drop the associated debug support code.

---
 src/log.c    | 10 +++-------
 src/log.h    |  3 +--
 src/plugin.c |  2 --
 src/plugin.h |  2 --
 4 files changed, 4 insertions(+), 13 deletions(-)

index 0155a6bba..db855d2b1 100644
@@ -138,15 +138,11 @@ static gboolean is_enabled(struct btd_debug_desc *desc)
 	return 0;
 }
 
-void __btd_enable_debug(struct btd_debug_desc *start,
-					struct btd_debug_desc *stop)
+void __btd_enable_debug(void)
 {
 	struct btd_debug_desc *desc;
 
-	if (start == NULL || stop == NULL)
-		return;
-
-	for (desc = start; desc < stop; desc++) {
+	for (desc = __start___debug; desc < __stop___debug; desc++) {
 		if (is_enabled(desc))
 			desc->flags |= BTD_DEBUG_FLAG_PRINT;
 	}

@@ -167,7 +163,7 @@ void __btd_log_init(const char *debug, int detach)
 	if (debug != NULL)
 		enabled = g_strsplit_set(debug, ":, ", 0);
 
-	__btd_enable_debug(__start___debug, __stop___debug);
+	__btd_enable_debug();
 
 	bt_log_open();
 

index 1ed742a0d..303fb36df 100644
@@ -36,8 +36,7 @@ struct btd_debug_desc {
 	unsigned int flags;
 } __attribute__((aligned(8)));
 
-void __btd_enable_debug(struct btd_debug_desc *start,
-					struct btd_debug_desc *stop);
+void __btd_enable_debug(void);
 
 /**
  * DBG:

index 1631f201c..e3eb12c0c 100644
@@ -62,8 +62,6 @@ static void add_plugin(struct bluetooth_plugin_desc *desc)
 	plugin->active = FALSE;
 	plugin->desc = desc;
 
-	__btd_enable_debug(desc->debug_start, desc->debug_stop);
-
 	plugins = g_slist_insert_sorted(plugins, plugin, compare_priority);
 }
 

index 7ff55e796..7a2d07b3d 100644
@@ -17,8 +17,6 @@ struct bluetooth_plugin_desc {
 	int priority;
 	int (*init) (void);
 	void (*exit) (void);
-	void *debug_start;
-	void *debug_stop;
 };
 
 #define BLUETOOTH_PLUGIN_DEFINE(name, version, priority, init, exit) \

2.43.0

From: Emil Velikov <[email protected]>

With the final external plugin gone, we can keep the API internal.

---
 Makefile.am       |  3 ---
 src/bluetooth.ver | 20 --------------------
 2 files changed, 23 deletions(-)

index 4db3a2953..c421f53c4 100644
@@ -291,7 +291,6 @@ pkglibexec_PROGRAMS += src/bluetoothd
 
 src_bluetoothd_SOURCES = $(builtin_sources) \
 			$(attrib_sources) $(btio_sources) \
-			src/bluetooth.ver \
 			src/main.c src/log.h src/log.c \
 			src/backtrace.h src/backtrace.c \
 			src/rfkill.c src/btd.h src/sdpd.h \

@@ -323,8 +322,6 @@ src_bluetoothd_LDADD = lib/libbluetooth-internal.la \
 			src/libshared-glib.la \
 			$(BACKTRACE_LIBS) $(GLIB_LIBS) $(DBUS_LIBS) -ldl -lrt \
 			$(builtin_ldadd)
-src_bluetoothd_LDFLAGS = $(AM_LDFLAGS) -Wl,--export-dynamic \
-				-Wl,--version-script=$(srcdir)/src/bluetooth.ver
 
 src_bluetoothd_DEPENDENCIES = lib/libbluetooth-internal.la \
 				gdbus/libgdbus-internal.la \

deleted file mode 100644
index a96fda2a1..000000000
@@ -1,20 +0,0 @@
-{
-	global:
-		btd_*;
-		g_dbus_*;
-		info;
-		error;
-		debug;
-		baswap;
-		ba2str;
-		/* Don't break LLVM sanitizers */
-		__asan*;
-		__dfsan*;
-		__lsan*;
-		__msan*;
-		__sanitizer*;
-		__tsan*;
-		__ubsan*;
-	local:
-		*;
-};

2.43.0

From: Emil Velikov <[email protected]>

The android specific modules, have a designated HMI entrypoint. Hide everything else with -fvisibility=hidden.

---
 android/Makefile.am     | 3 +++
 android/hal-audio.c     | 1 +
 android/hal-bluetooth.c | 1 +
 android/hal-sco.c       | 1 +
 4 files changed, 6 insertions(+)

index 309910147..e3756e89c 100644
@@ -96,6 +96,7 @@ android_bluetooth_default_la_SOURCES = android/hal.h android/hal-bluetooth.c \
 					android/hal-log.h \
 					android/hal-ipc.h android/hal-ipc.c \
 					android/hal-utils.h android/hal-utils.c
+android_bluetooth_default_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
 android_bluetooth_default_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/android
 android_bluetooth_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \
 					-no-undefined

@@ -195,6 +196,7 @@ android_audio_a2dp_default_la_SOURCES = android/audio-msg.h \
 					android/hardware/audio_effect.h \
 					android/hardware/hardware.h \
 					android/system/audio.h
+android_audio_a2dp_default_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
 android_audio_a2dp_default_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/android \
 					$(SBC_CFLAGS)
 android_audio_a2dp_default_la_LIBADD = $(SBC_LIBS) -lrt

@@ -212,6 +214,7 @@ android_audio_sco_default_la_SOURCES = android/hal-log.h \
 					android/audio_utils/resampler.c \
 					android/audio_utils/resampler.h \
 					android/system/audio.h
+android_audio_sco_default_la_CFLAGS = $(AM_CFLAGS) -fvisibility=hidden
 android_audio_sco_default_la_CPPFLAGS = $(AM_CPPFLAGS) -I$(srcdir)/android
 android_audio_sco_default_la_LIBADD = $(SPEEXDSP_LIBS) -lrt
 android_audio_sco_default_la_LDFLAGS = $(AM_LDFLAGS) -module -avoid-version \

index d37d6098c..f3d9b40a6 100644
@@ -1618,6 +1618,7 @@ static struct hw_module_methods_t hal_module_methods = {
 	.open = audio_open,
 };
 
+__attribute__ ((visibility("default")))
 struct audio_module HAL_MODULE_INFO_SYM = {
 	.common = {
 		.tag = HARDWARE_MODULE_TAG,

index d4442e620..7d1e5ac63 100644
@@ -1117,6 +1117,7 @@ static struct hw_module_methods_t bluetooth_module_methods = {
 	.open = open_bluetooth,
 };
 
+__attribute__ ((visibility("default")))
 struct hw_module_t HAL_MODULE_INFO_SYM = {
 	.tag = HARDWARE_MODULE_TAG,
 	.version_major = 1,

index d7c08a68b..3d66ad357 100644
@@ -1507,6 +1507,7 @@ static struct hw_module_methods_t hal_module_methods = {
 	.open = sco_open,
 };
 
+__attribute__ ((visibility("default")))
 struct audio_module HAL_MODULE_INFO_SYM = {
 	.common = {
 		.tag = HARDWARE_MODULE_TAG,

2.43.0

Hi Emil,

Im not so sure I want to remove the external plugins support completely, but I do understand that normally distros don't really want to have it enabled in production due to the reasons mentioned above, but I think we could find a middle ground here by disabling it by default but still let systems to re-enable it if they have some custom plugin that they may still want to use as external plugin.

Luiz Augusto von Dentz

Thanks for the feedback. Sure, I can convert this to a "configure --support-external-plugins", where all the presently removed code will be compiled out.

Still not entirely sure how external plugins are supposed to work, considering the API/ABI mentioned earlier - any pointers? Do you know of any example plugins that you can mention?

-Emil

Hi Emil,

Not really, well just the sixaxis but that can be converted to be built-in, but that being external means that there could be external plugins we don't know about thus why I think we would be better to have a flag to re-enable them just in case. Anyway I think for sixaxis we could just have it as built-in since it seems popular enough with the likes of steam deck, etc.

Luiz Augusto von Dentz

Ack, much appreciated. Will send v2 shortly.

Emil