Hi Archie,
On Wed, Feb 26, 2020 at 12:14 AM Archie Pusaka <apusaka@xxxxxxxxxx> wrote:
>
> From: Archie Pusaka <apusaka@xxxxxxxxxxxx>
>
> According to bluetooth spec Ver 5.1, Vol 3, Part B, 4.7.2, there
> might be multiple service records returned in a SDP Service Search
> Attribute Response. Also, according to 2.5.2, the service pattern
> can match any UUID contained within the service record, it doesn't
> have to match only some specific attributes of the record.
>
> Therefore, before using the service record to connect to any
> profile, first we must check that the service class ID of the
> service record matches with whatever UUID specified in the service
> pattern we are looking for.
>
> This patch checks the service class ID of the records against the
> requested UUID whenever bt_search_service() is called and filter
> out the ones that don't match. For the alternative where filtering
> is not applied, use the method bt_search().
> ---
>
> Changes in v2:
> - Move service class matching from profile.c to sdp_client.c
> - Create function bt_search for searching without matching uuid
> - Update device.c to use bt_search for L2CAP
>
> src/device.c | 17 ++++---------
> src/sdp-client.c | 62 ++++++++++++++++++++++++++++++++++++++++++++----
> src/sdp-client.h | 3 +++
> 3 files changed, 64 insertions(+), 18 deletions(-)
>
> diff --git a/src/device.c b/src/device.c
> index a8f4c22f3..5ff381959 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -4590,17 +4590,8 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs)
> if (!rec)
> break;
>
> - if (sdp_get_service_classes(rec, &svcclass) < 0)
> - continue;
> -
> - /* Check for empty service classes list */
> - if (svcclass == NULL) {
> - DBG("Skipping record with no service classes");
> - continue;
> - }
This actually still need to be checked since there could be malformed
records which don't set a service class, that said perhaps we could
deal with that in sdp-client.c but it doesn't seem you have added
these checks there.
> + profile_uuid = bt_uuid2string(&rec->svclass);
>
> - /* Extract the first element and skip the remainning */
> - profile_uuid = bt_uuid2string(svcclass->data);
> if (!profile_uuid) {
> sdp_list_free(svcclass, free);
> continue;
> @@ -5352,9 +5343,9 @@ static int device_browse_sdp(struct btd_device *device, DBusMessage *msg)
>
> req->sdp_flags = get_sdp_flags(device);
>
> - err = bt_search_service(btd_adapter_get_address(adapter),
> - &device->bdaddr, &uuid, browse_cb, req, NULL,
> - req->sdp_flags);
> + err = bt_search(btd_adapter_get_address(adapter),
> + &device->bdaddr, &uuid, browse_cb, req, NULL,
> + req->sdp_flags);
> if (err < 0) {
> browse_request_free(req);
> return err;
> diff --git a/src/sdp-client.c b/src/sdp-client.c
> index 413cf30ec..fc8e9ec10 100644
> --- a/src/sdp-client.c
> +++ b/src/sdp-client.c
> @@ -143,6 +143,7 @@ struct search_context {
> gpointer user_data;
> uuid_t uuid;
> guint io_id;
> + gboolean filter_svc_class;
> };
>
> static GSList *context_list = NULL;
> @@ -157,6 +158,11 @@ static void search_context_cleanup(struct search_context *ctxt)
> g_free(ctxt);
> }
>
> +static gboolean check_record_uuid(sdp_record_t *rec, uuid_t *uuid)
> +{
> + return sdp_uuid_cmp(uuid, &rec->svclass) == 0;
> +}
I wouldn't bother adding this function, instead just use sdp_uuid_cmp directly.
> static void search_completed_cb(uint8_t type, uint16_t status,
> uint8_t *rsp, size_t size, void *user_data)
> {
> @@ -195,6 +201,12 @@ static void search_completed_cb(uint8_t type, uint16_t status,
> rsp += recsize;
> bytesleft -= recsize;
>
> + if (ctxt->filter_svc_class &&
> + !check_record_uuid(rec, &ctxt->uuid)) {
> + sdp_record_free(rec);
> + continue;
> + }
> +
> recs = sdp_list_append(recs, rec);
> } while (scanned < (ssize_t) size && bytesleft > 0);
>
> @@ -338,7 +350,28 @@ static int create_search_context(struct search_context **ctxt,
> return 0;
> }
>
> -int bt_search_service(const bdaddr_t *src, const bdaddr_t *dst,
> +static int create_search_context_full(struct search_context **ctxt,
> + const bdaddr_t *src,
> + const bdaddr_t *dst,
> + uuid_t *uuid, uint16_t flags,
> + void *user_data, bt_callback_t cb,
> + bt_destroy_t destroy,
> + gboolean filter_svc_class)
> +{
> + int err = create_search_context(ctxt, src, dst, uuid, flags);
> +
> + if (err < 0)
> + return err;
> +
> + (*ctxt)->cb = cb;
> + (*ctxt)->destroy = destroy;
> + (*ctxt)->user_data = user_data;
> + (*ctxt)->filter_svc_class = filter_svc_class;
> +
> + return 0;
> +}
> +
> +int bt_search(const bdaddr_t *src, const bdaddr_t *dst,
> uuid_t *uuid, bt_callback_t cb, void *user_data,
> bt_destroy_t destroy, uint16_t flags)
> {
> @@ -348,13 +381,32 @@ int bt_search_service(const bdaddr_t *src, const bdaddr_t *dst,
> if (!cb)
> return -EINVAL;
>
> - err = create_search_context(&ctxt, src, dst, uuid, flags);
> + /* The resulting service class ID doesn't have to match uuid */
> + err = create_search_context_full(&ctxt, src, dst, uuid, flags,
> + user_data, cb, destroy, FALSE);
> if (err < 0)
> return err;
>
> - ctxt->cb = cb;
> - ctxt->destroy = destroy;
> - ctxt->user_data = user_data;
> + context_list = g_slist_append(context_list, ctxt);
> +
> + return 0;
> +}
> +
> +int bt_search_service(const bdaddr_t *src, const bdaddr_t *dst,
> + uuid_t *uuid, bt_callback_t cb, void *user_data,
> + bt_destroy_t destroy, uint16_t flags)
> +{
> + struct search_context *ctxt = NULL;
> + int err;
> +
> + if (!cb)
> + return -EINVAL;
> +
> + /* The resulting service class ID need to match uuid */
> + err = create_search_context_full(&ctxt, src, dst, uuid, flags,
> + user_data, cb, destroy, TRUE);
> + if (err < 0)
> + return err;
>
> context_list = g_slist_append(context_list, ctxt);
>
> diff --git a/src/sdp-client.h b/src/sdp-client.h
> index 9aa5a4d98..3a7212fd2 100644
> --- a/src/sdp-client.h
> +++ b/src/sdp-client.h
> @@ -24,6 +24,9 @@
> typedef void (*bt_callback_t) (sdp_list_t *recs, int err, gpointer user_data);
> typedef void (*bt_destroy_t) (gpointer user_data);
>
> +int bt_search(const bdaddr_t *src, const bdaddr_t *dst,
> + uuid_t *uuid, bt_callback_t cb, void *user_data,
> + bt_destroy_t destroy, uint16_t flags);
> int bt_search_service(const bdaddr_t *src, const bdaddr_t *dst,
> uuid_t *uuid, bt_callback_t cb, void *user_data,
> bt_destroy_t destroy, uint16_t flags);
> --
> 2.25.1.481.gfbce0eb801-goog
Other than that it looks pretty good.
--
Luiz Augusto von Dentz
