Hi Archie, On Sun, Mar 1, 2020 at 8:07 PM 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 v3: > - Simplify sdp_client.c > - Add comment for the behavior when SDP record is missing the > service class ID > > 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 | 26 +++++++-------------- > src/sdp-client.c | 61 ++++++++++++++++++++++++++++++++++++++++++++---- > src/sdp-client.h | 3 +++ > 3 files changed, 67 insertions(+), 23 deletions(-) > > diff --git a/src/device.c b/src/device.c > index a8f4c22f3..c351a1562 100644 > --- a/src/device.c > +++ b/src/device.c > @@ -4584,27 +4584,18 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs) > > for (seq = recs; seq; seq = seq->next) { > sdp_record_t *rec = (sdp_record_t *) seq->data; > - sdp_list_t *svcclass = NULL; > char *profile_uuid; > > 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; > - } > + /* If service class attribute is missing, svclass will be all > + * zero and the resulting uuid string will be NULL. > + */ > + 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); > + if (!profile_uuid) > continue; > - } > > if (bt_uuid_strcmp(profile_uuid, PNP_UUID) == 0) { > uint16_t source, vendor, product, version; > @@ -4638,7 +4629,6 @@ static void update_bredr_services(struct browse_req *req, sdp_list_t *recs) > > next: > free(profile_uuid); > - sdp_list_free(svcclass, free); > } > > if (sdp_key_file) { > @@ -5352,9 +5342,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..b92a083af 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; > @@ -195,6 +196,16 @@ static void search_completed_cb(uint8_t type, uint16_t status, > rsp += recsize; > bytesleft -= recsize; > > + /* Check whether service class ID matches some specified uuid. > + * If the record is missing service class ID, svclass will be > + * all zero, and thus will be unequal to the requested uuid. > + */ > + if (ctxt->filter_svc_class && > + sdp_uuid_cmp(&ctxt->uuid, &rec->svclass) != 0) { > + sdp_record_free(rec); > + continue; > + } > + > recs = sdp_list_append(recs, rec); > } while (scanned < (ssize_t) size && bytesleft > 0); > > @@ -338,7 +349,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 +380,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.0.265.gbab2e86ba0-goog Applied, thanks. -- Luiz Augusto von Dentz