[v2] adapter: Cancel the service authorization when remote is disconnected

If the remote device drops the connection before DUT confirm the
service authorization, the DUT still must wait for service
authorization timeout before processing future request.

Cancel the service authorization request when connection is dropped.
---
 src/adapter.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

Hi Cheng,

On Mon, Aug 26, 2024 at 10:30 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
>
> Hi Luiz,
>
> Thank you for your feedback. Please check the comment inline.
>
> On 8/26/2024 9:43 PM, Luiz Augusto von Dentz wrote:
> > Hi Cheng,
> >
> > On Mon, Aug 26, 2024 at 5:01 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
> >>
> >> If the remote device drops the connection before DUT confirm the
> >> service authorization, the DUT still must wait for service
> >> authorization timeout before processing future request.
> >>
> >> Cancel the service authorization request when connection is dropped.
> >> ---
> >>  src/adapter.c | 20 ++++++++++++++++++++
> >>  1 file changed, 20 insertions(+)
> >>
> >> diff --git a/src/adapter.c b/src/adapter.c
> >> index 245de4456..3ad000425 100644
> >> --- a/src/adapter.c
> >> +++ b/src/adapter.c
> >> @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason)
> >>         }
> >>  }
> >>
> >> +static void adapter_cancel_service_auth(struct btd_adapter *adapter,
> >> +                                       struct btd_device *device)
> >> +{
> >> +       struct service_auth *auth;
> >> +       GList *l;
> >> +
> >> +       auth = NULL;
> >> +       for (l = adapter->auths->head; l != NULL; l = l->next) {
> >> +               auth = l->data;
> >> +               if (auth->device == device)
> >> +                       break;
> >> +       }
> >> +       if (auth != NULL) {
> >> +               DBG("Cancel pending auth: %s", auth->uuid);
> >> +               g_queue_remove(auth->adapter->auths, auth);
> >> +               service_auth_cancel(auth);
> >> +       }
> >> +}
> >> +
> >>  static void dev_disconnected(struct btd_adapter *adapter,
> >>                                         const struct mgmt_addr_info *addr,
> >>                                         uint8_t reason)
> >> @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter,
> >>         device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type);
> >>         if (device) {
> >>                 adapter_remove_connection(adapter, device, addr->type);
> >> +               adapter_cancel_service_auth(adapter, device);
> >
> > This shall probably be placed together with
> > device_cancel_authentication in adapter_remove_connection but we need
> > to track in what bearer the authorization is for and remove all
> > authorization requests like in btd_adapter_remove_device:
> Yes. It can be put in device_cancel_authentication, but the condition to call
> this function in adapter_remove_connection need change. device_is_authenticating
> is only true when request, notify or confirm pincode/passkey.
> As for bearer, the service authorize is only happened on BDADDR_BREDR. So it
> can be called when the address type is BDADDR_BREDR in dev_disconnected.
> >
> >     l = adapter->auths->head;
> >     while (l != NULL) {
> >         struct service_auth *auth = l->data;
> >         GList *next = g_list_next(l);
> >
> >         if (auth->device != dev) {
> >             l = next;
> >             continue;
> >         }
> >
> >         g_queue_delete_link(adapter->auths, l);
> >         l = next;
> >
> >         service_auth_cancel(auth);
> >    }
> >
> > I'd probably move the above code to a helper function so it can be
> > called from different places, as for doing this on a per bearer basis
> > would probably need to add bearer information to btd_service but I
> > guess that can be treated separately.
> Yes, it's great. Need use the above code to cancel all pending authorize.

Looks like you never send another version of this one, are you still
planning on having this fixed?

> >
> >
> >>                 disconnect_notify(device, reason);
> >>         }
> >>
> >> --
> >> 2.25.1
> >>
> >>
> >
> >
>

Hi Luiz,

Sorry for the delay, will send the new patch soon. 


On 9/20/2024 3:53 AM, Luiz Augusto von Dentz wrote:
> Hi Cheng,
> 
> On Mon, Aug 26, 2024 at 10:30 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
>>
>> Hi Luiz,
>>
>> Thank you for your feedback. Please check the comment inline.
>>
>> On 8/26/2024 9:43 PM, Luiz Augusto von Dentz wrote:
>>> Hi Cheng,
>>>
>>> On Mon, Aug 26, 2024 at 5:01 AM Cheng Jiang <quic_chejiang@quicinc.com> wrote:
>>>>
>>>> If the remote device drops the connection before DUT confirm the
>>>> service authorization, the DUT still must wait for service
>>>> authorization timeout before processing future request.
>>>>
>>>> Cancel the service authorization request when connection is dropped.
>>>> ---
>>>>  src/adapter.c | 20 ++++++++++++++++++++
>>>>  1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/src/adapter.c b/src/adapter.c
>>>> index 245de4456..3ad000425 100644
>>>> --- a/src/adapter.c
>>>> +++ b/src/adapter.c
>>>> @@ -8502,6 +8502,25 @@ static void disconnect_notify(struct btd_device *dev, uint8_t reason)
>>>>         }
>>>>  }
>>>>
>>>> +static void adapter_cancel_service_auth(struct btd_adapter *adapter,
>>>> +                                       struct btd_device *device)
>>>> +{
>>>> +       struct service_auth *auth;
>>>> +       GList *l;
>>>> +
>>>> +       auth = NULL;
>>>> +       for (l = adapter->auths->head; l != NULL; l = l->next) {
>>>> +               auth = l->data;
>>>> +               if (auth->device == device)
>>>> +                       break;
>>>> +       }
>>>> +       if (auth != NULL) {
>>>> +               DBG("Cancel pending auth: %s", auth->uuid);
>>>> +               g_queue_remove(auth->adapter->auths, auth);
>>>> +               service_auth_cancel(auth);
>>>> +       }
>>>> +}
>>>> +
>>>>  static void dev_disconnected(struct btd_adapter *adapter,
>>>>                                         const struct mgmt_addr_info *addr,
>>>>                                         uint8_t reason)
>>>> @@ -8516,6 +8535,7 @@ static void dev_disconnected(struct btd_adapter *adapter,
>>>>         device = btd_adapter_find_device(adapter, &addr->bdaddr, addr->type);
>>>>         if (device) {
>>>>                 adapter_remove_connection(adapter, device, addr->type);
>>>> +               adapter_cancel_service_auth(adapter, device);
>>>
>>> This shall probably be placed together with
>>> device_cancel_authentication in adapter_remove_connection but we need
>>> to track in what bearer the authorization is for and remove all
>>> authorization requests like in btd_adapter_remove_device:
>> Yes. It can be put in device_cancel_authentication, but the condition to call
>> this function in adapter_remove_connection need change. device_is_authenticating
>> is only true when request, notify or confirm pincode/passkey.
>> As for bearer, the service authorize is only happened on BDADDR_BREDR. So it
>> can be called when the address type is BDADDR_BREDR in dev_disconnected.
>>>
>>>     l = adapter->auths->head;
>>>     while (l != NULL) {
>>>         struct service_auth *auth = l->data;
>>>         GList *next = g_list_next(l);
>>>
>>>         if (auth->device != dev) {
>>>             l = next;
>>>             continue;
>>>         }
>>>
>>>         g_queue_delete_link(adapter->auths, l);
>>>         l = next;
>>>
>>>         service_auth_cancel(auth);
>>>    }
>>>
>>> I'd probably move the above code to a helper function so it can be
>>> called from different places, as for doing this on a per bearer basis
>>> would probably need to add bearer information to btd_service but I
>>> guess that can be treated separately.
>> Yes, it's great. Need use the above code to cancel all pending authorize.
> 
> Looks like you never send another version of this one, are you still
> planning on having this fixed?
> 
>>>
>>>
>>>>                 disconnect_notify(device, reason);
>>>>         }
>>>>
>>>> --
>>>> 2.25.1
>>>>
>>>>
>>>
>>>
>>
> 
>