2021-02-22 05:22:54

by Archie Pusaka

[permalink] [raw]
Subject: [Bluez PATCH] adapter: Check whether adapter is pending powered

From: Archie Pusaka <[email protected]>

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 <[email protected]>
---

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

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;
--
2.30.0.617.g56c4b15f3c-goog


2021-02-22 05:38:46

by bluez.test.bot

[permalink] [raw]
Subject: RE: [Bluez] adapter: Check whether adapter is pending powered

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

2021-02-22 18:29:26

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [Bluez] adapter: Check whether adapter is pending powered

Hi Archie,

On Sun, Feb 21, 2021 at 9:38 PM <[email protected]> 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.

--
Luiz Augusto von Dentz