[Bluez] adapter: Check whether adapter is pending powered

Commit Message

Archie Pusaka Feb. 22, 2021, 5:18 a.m. UTC

From: Archie Pusaka <[e-mail]>

The powered setting of adapter is frequently being checked, but the
check only accounts for whether it currently is powered. When powering
off the adapter, there is a brief moment when the adapter is already
off kernel-wise, but powered property is still true.

If powered property is accessed at this time, some problems might
occur. For example, if RemoveDevice DBus API is called, we would still
carry out the request and remove the device from the user space, but
we would fail to remove it from the kernel, therefore resulting an
error when we want to re-pair the device.

This patch addresses this issue by also checking whether adapter is
being powered off when checking for powered.

Reviewed-by: Sonny Sasaka <[e-mail]>
---

 src/adapter.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

[e-mail] Feb. 22, 2021, 5:35 a.m. UTC | #1

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

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth

Luiz Augusto von Dentz Feb. 22, 2021, 6:26 p.m. UTC | #2

Hi Archie,

On Sun, Feb 21, 2021 at 9:38 PM <[e-mail]> wrote:
>
> 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=436321
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - PASS
>
> ##############################
> Test: CheckGitLint - PASS
>
> ##############################
> Test: CheckBuild - PASS
>
> ##############################
> Test: MakeCheck - PASS
>
>
>
> ---
> Regards,
> Linux Bluetooth

Applied, thanks.

Patch

diff --git a/src/adapter.c b/src/adapter.c
index c1f976323e..fab01cee69 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -548,11 +548,10 @@  static void settings_changed(struct btd_adapter *adapter, uint32_t settings)
 		}
 	}
 
-	if (changed_mask & MGMT_SETTING_LE) {
-		if ((adapter->current_settings & MGMT_SETTING_POWERED) &&
+	if ((changed_mask & MGMT_SETTING_LE) &&
+				btd_adapter_get_powered(adapter) &&
 				(adapter->current_settings & MGMT_SETTING_LE))
-			trigger_passive_scanning(adapter);
-	}
+		trigger_passive_scanning(adapter);
 
 	if (changed_mask & MGMT_SETTING_DISCOVERABLE) {
 		g_dbus_emit_property_changed(dbus_conn, adapter->path,
@@ -1795,7 +1794,7 @@  static void trigger_start_discovery(struct btd_adapter *adapter, guint delay)
 	 *
 	 * This is safe-guard and should actually never trigger.
 	 */
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return;
 
 	adapter->discovery_idle_timeout = g_timeout_add_seconds(delay,
@@ -2286,7 +2285,7 @@  static DBusMessage *start_discovery(DBusConnection *conn,
 
 	DBG("sender %s", sender);
 
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return btd_error_not_ready(msg);
 
 	is_discovering = get_discovery_client(adapter, sender, &client);
@@ -2586,7 +2585,7 @@  static DBusMessage *set_discovery_filter(DBusConnection *conn,
 
 	DBG("sender %s", sender);
 
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return btd_error_not_ready(msg);
 
 	if (MGMT_VERSION(mgmt_version, mgmt_revision) < MGMT_VERSION(1, 8))
@@ -2643,7 +2642,7 @@  static DBusMessage *stop_discovery(DBusConnection *conn,
 
 	DBG("sender %s", sender);
 
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return btd_error_not_ready(msg);
 
 	list = g_slist_find_custom(adapter->discovery_list, sender,
@@ -2999,7 +2998,7 @@  static void property_set_discoverable(const GDBusPropertyTable *property,
 	struct btd_adapter *adapter = user_data;
 
 	if (adapter->discoverable_timeout > 0 &&
-			!(adapter->current_settings & MGMT_SETTING_POWERED)) {
+			!btd_adapter_get_powered(adapter)) {
 		g_dbus_pending_property_error(id, ERROR_INTERFACE ".Failed",
 								"Not Powered");
 		return;
@@ -3250,7 +3249,7 @@  static DBusMessage *remove_device(DBusConnection *conn,
 	if (!list)
 		return btd_error_does_not_exist(msg);
 
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return btd_error_not_ready(msg);
 
 	device = list->data;
@@ -3401,7 +3400,7 @@  static DBusMessage *connect_device(DBusConnection *conn,
 
 	DBG("sender %s", dbus_message_get_sender(msg));
 
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return btd_error_not_ready(msg);
 
 	dbus_message_iter_init(msg, &iter);
@@ -4802,7 +4801,8 @@  bool btd_adapter_get_pairable(struct btd_adapter *adapter)
 
 bool btd_adapter_get_powered(struct btd_adapter *adapter)
 {
-	if (adapter->current_settings & MGMT_SETTING_POWERED)
+	if ((adapter->current_settings & MGMT_SETTING_POWERED) &&
+			!(adapter->pending_settings & MGMT_SETTING_POWERED))
 		return true;
 
 	return false;
@@ -4893,7 +4893,7 @@  int adapter_connect_list_add(struct btd_adapter *adapter,
 							adapter->system_name);
 
 done:
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return 0;
 
 	trigger_passive_scanning(adapter);
@@ -4930,7 +4930,7 @@  void adapter_connect_list_remove(struct btd_adapter *adapter,
 		return;
 	}
 
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return;
 
 	trigger_passive_scanning(adapter);
@@ -7194,7 +7194,7 @@  int btd_cancel_authorization(guint id)
 
 int btd_adapter_restore_powered(struct btd_adapter *adapter)
 {
-	if (adapter->current_settings & MGMT_SETTING_POWERED)
+	if (btd_adapter_get_powered(adapter))
 		return 0;
 
 	set_mode(adapter, MGMT_OP_SET_POWERED, 0x01);
@@ -7229,7 +7229,7 @@  void btd_adapter_register_msd_cb(struct btd_adapter *adapter,
 int btd_adapter_set_fast_connectable(struct btd_adapter *adapter,
 							gboolean enable)
 {
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return -EINVAL;
 
 	set_mode(adapter, MGMT_OP_SET_FAST_CONNECTABLE, enable ? 0x01 : 0x00);
@@ -7241,7 +7241,7 @@  int btd_adapter_read_clock(struct btd_adapter *adapter, const bdaddr_t *bdaddr,
 				int which, int timeout, uint32_t *clock,
 				uint16_t *accuracy)
 {
-	if (!(adapter->current_settings & MGMT_SETTING_POWERED))
+	if (!btd_adapter_get_powered(adapter))
 		return -EINVAL;
 
 	return -ENOSYS;
@@ -8709,7 +8709,7 @@  load:
 
 	/* retrieve the active connections: address the scenario where
 	 * the are active connections before the daemon've started */
-	if (adapter->current_settings & MGMT_SETTING_POWERED)
+	if (btd_adapter_get_powered(adapter))
 		load_connections(adapter);
 
 	adapter->initialized = TRUE;
@@ -9507,7 +9507,7 @@  static void read_info_complete(uint8_t status, uint16_t length,
 	if (adapter->stored_discoverable && !adapter->discoverable_timeout)
 		set_discoverable(adapter, 0x01, 0);
 
-	if (adapter->current_settings & MGMT_SETTING_POWERED)
+	if (btd_adapter_get_powered(adapter))
 		adapter_start(adapter);
 
 	return;