* [Bluez PATCH v1] adapter: Remove temporary devices before power off
@ 2021-01-06 9:26 Archie Pusaka
2021-01-06 9:51 ` [Bluez,v1] " bluez.test.bot
2021-01-06 18:24 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
0 siblings, 2 replies; 4+ messages in thread
From: Archie Pusaka @ 2021-01-06 9:26 UTC (permalink / raw)
To: linux-bluetooth, Luiz Augusto von Dentz; +Cc: CrosBT Upstreaming, Archie Pusaka
From: Archie Pusaka <apusaka@chromium.org>
If adapter is powered off when a currently connected device is
being removed, there is a possibility that we haven't finish waiting
for the disconnection but the adapter is already powered down.
When this happens, the kernel would fail to clean the device's
information, for example the pairing information. This causes
disagreement between the user space and the kernel about whether the
device is already paired, because the device is successfully removed
from the user space's perspective.
This patch enforces the removal of such devices before allowing the
adapter to power off.
---
src/adapter.c | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/src/adapter.c b/src/adapter.c
index ec6a6a64c5..92d1cb2232 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -517,6 +517,7 @@ static void adapter_stop(struct btd_adapter *adapter);
static void trigger_passive_scanning(struct btd_adapter *adapter);
static bool set_mode(struct btd_adapter *adapter, uint16_t opcode,
uint8_t mode);
+static void remove_temporary_devices(struct btd_adapter *adapter);
static void settings_changed(struct btd_adapter *adapter, uint32_t settings)
{
@@ -622,6 +623,8 @@ static bool set_mode(struct btd_adapter *adapter, uint16_t opcode,
switch (mode) {
case MGMT_OP_SET_POWERED:
setting = MGMT_SETTING_POWERED;
+ if (!mode)
+ remove_temporary_devices(adapter);
break;
case MGMT_OP_SET_CONNECTABLE:
setting = MGMT_SETTING_CONNECTABLE;
@@ -2888,8 +2891,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
param = &mode;
len = sizeof(mode);
- if (!mode)
+ if (!mode) {
clear_discoverable(adapter);
+ remove_temporary_devices(adapter);
+ }
break;
case MGMT_SETTING_DISCOVERABLE:
@@ -5304,6 +5309,19 @@ static void remove_discovery_list(struct btd_adapter *adapter)
adapter->discovery_list = NULL;
}
+static void remove_temporary_devices(struct btd_adapter *adapter)
+{
+ GSList *l, *next;
+
+ for (l = adapter->devices; l; l = next) {
+ struct btd_device *dev = l->data;
+
+ next = g_slist_next(l);
+ if (device_is_temporary(dev))
+ btd_adapter_remove_device(adapter, dev);
+ }
+}
+
static void adapter_free(gpointer user_data)
{
struct btd_adapter *adapter = user_data;
--
2.29.2.729.g45daf8777d-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [Bluez,v1] adapter: Remove temporary devices before power off
2021-01-06 9:26 [Bluez PATCH v1] adapter: Remove temporary devices before power off Archie Pusaka
@ 2021-01-06 9:51 ` bluez.test.bot
2021-01-06 18:24 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2021-01-06 9:51 UTC (permalink / raw)
To: linux-bluetooth, apusaka
[-- Attachment #1: Type: text/plain, Size: 557 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=409877
---Test result---
##############################
Test: CheckPatch - PASS
##############################
Test: CheckGitLint - PASS
##############################
Test: CheckBuild - PASS
##############################
Test: MakeCheck - PASS
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bluez PATCH v1] adapter: Remove temporary devices before power off
2021-01-06 9:26 [Bluez PATCH v1] adapter: Remove temporary devices before power off Archie Pusaka
2021-01-06 9:51 ` [Bluez,v1] " bluez.test.bot
@ 2021-01-06 18:24 ` Luiz Augusto von Dentz
2021-01-06 19:05 ` Luiz Augusto von Dentz
1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-06 18:24 UTC (permalink / raw)
To: Archie Pusaka; +Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka
Hi Archie,
On Wed, Jan 6, 2021 at 1:27 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> If adapter is powered off when a currently connected device is
> being removed, there is a possibility that we haven't finish waiting
> for the disconnection but the adapter is already powered down.
>
> When this happens, the kernel would fail to clean the device's
> information, for example the pairing information. This causes
> disagreement between the user space and the kernel about whether the
> device is already paired, because the device is successfully removed
> from the user space's perspective.
>
> This patch enforces the removal of such devices before allowing the
> adapter to power off.
> ---
>
> src/adapter.c | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index ec6a6a64c5..92d1cb2232 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -517,6 +517,7 @@ static void adapter_stop(struct btd_adapter *adapter);
> static void trigger_passive_scanning(struct btd_adapter *adapter);
> static bool set_mode(struct btd_adapter *adapter, uint16_t opcode,
> uint8_t mode);
> +static void remove_temporary_devices(struct btd_adapter *adapter);
I would have the function above declared just before the first user
that way we don't have to use a forward declaration like above.
> static void settings_changed(struct btd_adapter *adapter, uint32_t settings)
> {
> @@ -622,6 +623,8 @@ static bool set_mode(struct btd_adapter *adapter, uint16_t opcode,
> switch (mode) {
> case MGMT_OP_SET_POWERED:
> setting = MGMT_SETTING_POWERED;
> + if (!mode)
> + remove_temporary_devices(adapter);
> break;
> case MGMT_OP_SET_CONNECTABLE:
> setting = MGMT_SETTING_CONNECTABLE;
> @@ -2888,8 +2891,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
> param = &mode;
> len = sizeof(mode);
>
> - if (!mode)
> + if (!mode) {
> clear_discoverable(adapter);
> + remove_temporary_devices(adapter);
> + }
>
> break;
> case MGMT_SETTING_DISCOVERABLE:
> @@ -5304,6 +5309,19 @@ static void remove_discovery_list(struct btd_adapter *adapter)
> adapter->discovery_list = NULL;
> }
>
> +static void remove_temporary_devices(struct btd_adapter *adapter)
> +{
> + GSList *l, *next;
> +
> + for (l = adapter->devices; l; l = next) {
> + struct btd_device *dev = l->data;
> +
> + next = g_slist_next(l);
> + if (device_is_temporary(dev))
> + btd_adapter_remove_device(adapter, dev);
> + }
> +}
> +
> static void adapter_free(gpointer user_data)
> {
> struct btd_adapter *adapter = user_data;
> --
> 2.29.2.729.g45daf8777d-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Bluez PATCH v1] adapter: Remove temporary devices before power off
2021-01-06 18:24 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
@ 2021-01-06 19:05 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2021-01-06 19:05 UTC (permalink / raw)
To: Archie Pusaka; +Cc: linux-bluetooth, CrosBT Upstreaming, Archie Pusaka
Hi Archie,
On Wed, Jan 6, 2021 at 10:24 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Archie,
>
> On Wed, Jan 6, 2021 at 1:27 AM Archie Pusaka <apusaka@google.com> wrote:
> >
> > From: Archie Pusaka <apusaka@chromium.org>
> >
> > If adapter is powered off when a currently connected device is
> > being removed, there is a possibility that we haven't finish waiting
> > for the disconnection but the adapter is already powered down.
> >
> > When this happens, the kernel would fail to clean the device's
> > information, for example the pairing information. This causes
> > disagreement between the user space and the kernel about whether the
> > device is already paired, because the device is successfully removed
> > from the user space's perspective.
> >
> > This patch enforces the removal of such devices before allowing the
> > adapter to power off.
> > ---
> >
> > src/adapter.c | 20 +++++++++++++++++++-
> > 1 file changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index ec6a6a64c5..92d1cb2232 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -517,6 +517,7 @@ static void adapter_stop(struct btd_adapter *adapter);
> > static void trigger_passive_scanning(struct btd_adapter *adapter);
> > static bool set_mode(struct btd_adapter *adapter, uint16_t opcode,
> > uint8_t mode);
> > +static void remove_temporary_devices(struct btd_adapter *adapter);
>
> I would have the function above declared just before the first user
> that way we don't have to use a forward declaration like above.
>
> > static void settings_changed(struct btd_adapter *adapter, uint32_t settings)
> > {
> > @@ -622,6 +623,8 @@ static bool set_mode(struct btd_adapter *adapter, uint16_t opcode,
> > switch (mode) {
> > case MGMT_OP_SET_POWERED:
> > setting = MGMT_SETTING_POWERED;
> > + if (!mode)
> > + remove_temporary_devices(adapter);
> > break;
> > case MGMT_OP_SET_CONNECTABLE:
> > setting = MGMT_SETTING_CONNECTABLE;
> > @@ -2888,8 +2891,10 @@ static void property_set_mode(struct btd_adapter *adapter, uint32_t setting,
> > param = &mode;
> > len = sizeof(mode);
> >
> > - if (!mode)
> > + if (!mode) {
> > clear_discoverable(adapter);
> > + remove_temporary_devices(adapter);
> > + }
> >
> > break;
> > case MGMT_SETTING_DISCOVERABLE:
> > @@ -5304,6 +5309,19 @@ static void remove_discovery_list(struct btd_adapter *adapter)
> > adapter->discovery_list = NULL;
> > }
> >
> > +static void remove_temporary_devices(struct btd_adapter *adapter)
> > +{
> > + GSList *l, *next;
> > +
> > + for (l = adapter->devices; l; l = next) {
> > + struct btd_device *dev = l->data;
> > +
> > + next = g_slist_next(l);
> > + if (device_is_temporary(dev))
> > + btd_adapter_remove_device(adapter, dev);
> > + }
> > +}
> > +
> > static void adapter_free(gpointer user_data)
> > {
> > struct btd_adapter *adapter = user_data;
> > --
> > 2.29.2.729.g45daf8777d-goog
> >
>
>
> --
> Luiz Augusto von Dentz
Applied, thanks. I went ahead and did the changes myself.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-06 19:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-06 9:26 [Bluez PATCH v1] adapter: Remove temporary devices before power off Archie Pusaka
2021-01-06 9:51 ` [Bluez,v1] " bluez.test.bot
2021-01-06 18:24 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
2021-01-06 19:05 ` Luiz Augusto von Dentz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).