[PATCH BlueZ 0/1] [v2] ExcludeAdapter configuration setting for input plugin

[PATCH BlueZ 0/1] [v2] ExcludeAdapter configuration setting for input plugin

[shwoseph]

As a bluez user I have run into cases where the input plugin can be problematic because it binds both HID PSMs on all bluetooth adapters. Simply disabling the plugin is not an ideal solution if you want to, for example, run an application that binds PSMs 17 and 19 on adapterA while using a bluetooth input device on adapterB. This proposed feature would allow users to determine which of their adapters can be engaged by the input plugin. Using the ExcludeAdapters key in input.conf a comma separated list of bdaddrs can be specified that the input plugin will not start a server on.

shwoseph (1):
  ExcludeAdapter configuration setting for input profile

 profiles/input/device.c   | 22 ++++++++++++++++++++++
 profiles/input/device.h   |  4 ++++
 profiles/input/input.conf |  4 ++++
 profiles/input/manager.c  | 34 +++++++++++++++++++++++++++++++++-
 4 files changed, 63 insertions(+), 1 deletion(-)

-- 
2.32.0

[PATCH BlueZ 1/1] [v2] ExcludeAdapter configuration setting for input profile

[shwoseph]

---
 profiles/input/device.c   | 22 ++++++++++++++++++++++
 profiles/input/device.h   |  4 ++++
 profiles/input/input.conf |  4 ++++
 profiles/input/manager.c  | 34 +++++++++++++++++++++++++++++++++-
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index e2ac6ea60..0192e7977 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -82,6 +82,8 @@ struct input_device {
 static int idle_timeout = 0;
 static bool uhid_enabled = false;
 static bool classic_bonded_only = false;
+static char **exclude_adapters;
+static gsize num_exclude_adapters;
 
 void input_set_idle_timeout(int timeout)
 {
@@ -103,6 +105,26 @@ bool input_get_classic_bonded_only(void)
 	return classic_bonded_only;
 }
 
+char **input_get_exclude_adapters(void)
+{
+	return exclude_adapters;
+}
+
+void input_set_exclude_adapters(char **adapters)
+{
+	exclude_adapters = adapters;
+}
+
+gsize input_get_num_exclude_adapters(void)
+{
+	return num_exclude_adapters;
+}
+
+void input_set_num_exclude_adapters(gsize num)
+{
+	num_exclude_adapters = num;
+}
+
 static void input_device_enter_reconnect_mode(struct input_device *idev);
 static int connection_disconnect(struct input_device *idev, uint32_t flags);
 static int uhid_disconnect(struct input_device *idev);
diff --git a/profiles/input/device.h b/profiles/input/device.h
index cf0389417..04fe41e2d 100644
--- a/profiles/input/device.h
+++ b/profiles/input/device.h
@@ -19,6 +19,10 @@ void input_enable_userspace_hid(bool state);
 void input_set_classic_bonded_only(bool state);
 bool input_get_classic_bonded_only(void);
 void input_set_auto_sec(bool state);
+char **input_get_exclude_adapters(void);
+void input_set_exclude_adapters(char **address);
+gsize input_get_num_exclude_adapters(void);
+void input_set_num_exclude_adapters(gsize address);
 
 int input_device_register(struct btd_service *service);
 void input_device_unregister(struct btd_service *service);
diff --git a/profiles/input/input.conf b/profiles/input/input.conf
index 4c70bc561..c8ec5ee30 100644
--- a/profiles/input/input.conf
+++ b/profiles/input/input.conf
@@ -24,3 +24,7 @@
 # Enables upgrades of security automatically if required.
 # Defaults to true to maximize device compatibility.
 #LEAutoSecurity=true
+
+# Exclude adapters
+# Disables input plugin on adapters with specified bdaddrs
+#ExcludeAdapters=00:00:00:00:00:00,00:00:00:00:00:01
diff --git a/profiles/input/manager.c b/profiles/input/manager.c
index 92789a003..ca687b726 100644
--- a/profiles/input/manager.c
+++ b/profiles/input/manager.c
@@ -32,7 +32,23 @@
 
 static int hid_server_probe(struct btd_profile *p, struct btd_adapter *adapter)
 {
-	return server_start(btd_adapter_get_address(adapter));
+	const bdaddr_t *address;
+	char addr[18];
+	char **exclude_adapters;
+	gsize num_exclude_adapters;
+	
+	address = btd_adapter_get_address(adapter);
+	ba2str(address, addr);
+	exclude_adapters = input_get_exclude_adapters();
+	num_exclude_adapters = input_get_num_exclude_adapters();
+
+	for (gsize i = 0; i < num_exclude_adapters; i++) {
+		if (strcmp(addr, exclude_adapters[i])) {
+			return 0;
+		}
+	}
+	
+	return server_start(address);
 }
 
 static void hid_server_remove(struct btd_profile *p,
@@ -83,6 +99,9 @@ static int input_init(void)
 	config = load_config_file(CONFIGDIR "/input.conf");
 	if (config) {
 		int idle_timeout;
+		char *exclude_adapters_str;
+		char **exclude_adapters;
+		gsize num_exclude_adapters;
 		gboolean uhid_enabled, classic_bonded_only, auto_sec;
 
 		idle_timeout = g_key_file_get_integer(config, "General",
@@ -121,6 +140,19 @@ static int input_init(void)
 		} else
 			g_clear_error(&err);
 
+		g_key_file_set_list_separator(config, ',');
+		
+		exclude_adapters_str = g_key_file_get_string(config, "General",
+				"ExcludeAdapters", &err);
+		exclude_adapters = g_key_file_get_string_list(config, "General",
+				"ExcludeAdapters", &num_exclude_adapters, &err);
+
+		if (!err) {
+			DBG("input.conf: ExcludeAdapters=%s", exclude_adapters_str);
+			input_set_exclude_adapters(exclude_adapters);
+			input_set_num_exclude_adapters(num_exclude_adapters);
+		} else
+			g_clear_error(&err);
 	}
 
 	btd_profile_register(&input_profile);
-- 
2.32.0

RE: ExcludeAdapter configuration setting for input plugin

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

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=658204

---Test result---

Test Summary:
CheckPatch                    FAIL      0.78 seconds
GitLint                       PASS      0.45 seconds
Prep - Setup ELL              PASS      31.19 seconds
Build - Prep                  PASS      0.78 seconds
Build - Configure             PASS      9.53 seconds
Build - Make                  PASS      1033.85 seconds
Make Check                    PASS      11.76 seconds
Make Check w/Valgrind         PASS      325.07 seconds
Make Distcheck                PASS      270.26 seconds
Build w/ext ELL - Configure   PASS      9.60 seconds
Build w/ext ELL - Make        PASS      95.31 seconds
Incremental Build w/ patches  PASS      0.00 seconds
Scan Build                    PASS      601.49 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script with rule in .checkpatch.conf
Output:
[BlueZ,1/1,v2] ExcludeAdapter configuration setting for input profile
ERROR:TRAILING_WHITESPACE: trailing whitespace
#167: FILE: profiles/input/manager.c:39:
+^I$

WARNING:BRACES: braces {} are not necessary for single statement blocks
#174: FILE: profiles/input/manager.c:46:
+		if (strcmp(addr, exclude_adapters[i])) {
+			return 0;
+		}

ERROR:TRAILING_WHITESPACE: trailing whitespace
#178: FILE: profiles/input/manager.c:50:
+^I$

ERROR:TRAILING_WHITESPACE: trailing whitespace
#198: FILE: profiles/input/manager.c:144:
+^I^I$

WARNING:LONG_LINE: line length of 84 exceeds 80 columns
#205: FILE: profiles/input/manager.c:151:
+			DBG("input.conf: ExcludeAdapters=%s", exclude_adapters_str);

/github/workspace/src/12912299.patch total: 3 errors, 2 warnings, 103 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

NOTE: Whitespace errors detected.
      You may wish to use scripts/cleanpatch or scripts/cleanfile

/github/workspace/src/12912299.patch has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.




---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ 0/1] [v2] ExcludeAdapter configuration setting for input plugin

[Luiz Augusto von Dentz]

Hi,

On Sat, Jul 9, 2022 at 11:38 AM shwoseph <shwoseph@gmail.com> wrote:
>
> As a bluez user I have run into cases where the input plugin can be problematic because it binds both HID PSMs on all bluetooth adapters. Simply disabling the plugin is not an ideal solution if you want to, for example, run an application that binds PSMs 17 and 19 on adapterA while using a bluetooth input device on adapterB. This proposed feature would allow users to determine which of their adapters can be engaged by the input plugin. Using the ExcludeAdapters key in input.conf a comma separated list of bdaddrs can be specified that the input plugin will not start a server on.
>
> shwoseph (1):
>   ExcludeAdapter configuration setting for input profile
>
>  profiles/input/device.c   | 22 ++++++++++++++++++++++
>  profiles/input/device.h   |  4 ++++
>  profiles/input/input.conf |  4 ++++
>  profiles/input/manager.c  | 34 +++++++++++++++++++++++++++++++++-
>  4 files changed, 63 insertions(+), 1 deletion(-)
>
> --
> 2.32.0

Well those PSM would require root to bind anyway so if the intention
is to allow implementing something like an HID device perhaps we would
better off allowing RegisterProfile with HID UUID that way the daemon
still have control of these PSMs but it can dispatch the connections
to an application that has registered:

https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt

On the plus side you can register an SDP record for it, but we may
need to do some changes if we want profile to be registered only to a
specific adapter.

-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ 0/1] [v2] ExcludeAdapter configuration setting for input plugin

[Luiz Augusto von Dentz]

Hi Shawn,

On Tue, Jul 12, 2022 at 12:35 PM Shawn J <shwoseph@gmail.com> wrote:
>
> I could see going that route, but doesn’t the input plugin register the hid profile on startup? Would RegisterProfile be able to seize the hid profile from the plugin at that point? If it can, I think this would solve the problem as long as it works with sdp. I’m still a little fuzzy on gatt/sdp and I thought profiles were more of a gatt thing.

We may need to do some changes, for instance it is probably better to
make input plugin register its profiles within src/profile.c like we
do with externa profiles, in fact this would probably be a nice thing
to do for every profile actually, so we have a src/profile.c managing
all our sockets instead of each plugin doing its own thing. Regarding
profiles, LE and Classic use the same term but they mean different
things, in LE there are GATT Services (which include attributes, etc)
and then a profile which normally consist of client procedures, on the
other hand in Classic the term Profile is used for both client and
server and uses SDP to declare it, RegisterProfile is actually meant
for Classic/SDP registration while in LE we have RegisterApplication
which has its own object tree to represent GATT services and
attributes.

> On Tue, Jul 12, 2022 at 14:18 Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote:
>>
>> Hi,
>>
>> On Sat, Jul 9, 2022 at 11:38 AM shwoseph <shwoseph@gmail.com> wrote:
>> >
>> > As a bluez user I have run into cases where the input plugin can be problematic because it binds both HID PSMs on all bluetooth adapters. Simply disabling the plugin is not an ideal solution if you want to, for example, run an application that binds PSMs 17 and 19 on adapterA while using a bluetooth input device on adapterB. This proposed feature would allow users to determine which of their adapters can be engaged by the input plugin. Using the ExcludeAdapters key in input.conf a comma separated list of bdaddrs can be specified that the input plugin will not start a server on.
>> >
>> > shwoseph (1):
>> >   ExcludeAdapter configuration setting for input profile
>> >
>> >  profiles/input/device.c   | 22 ++++++++++++++++++++++
>> >  profiles/input/device.h   |  4 ++++
>> >  profiles/input/input.conf |  4 ++++
>> >  profiles/input/manager.c  | 34 +++++++++++++++++++++++++++++++++-
>> >  4 files changed, 63 insertions(+), 1 deletion(-)
>> >
>> > --
>> > 2.32.0
>>
>> Well those PSM would require root to bind anyway so if the intention
>> is to allow implementing something like an HID device perhaps we would
>> better off allowing RegisterProfile with HID UUID that way the daemon
>> still have control of these PSMs but it can dispatch the connections
>> to an application that has registered:
>>
>> https://git.kernel.org/pub/scm/bluetooth/bluez.git/tree/doc/profile-api.txt
>>
>> On the plus side you can register an SDP record for it, but we may
>> need to do some changes if we want profile to be registered only to a
>> specific adapter.
>>
>> --
>> Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz