[PATCH BlueZ 1/5] lib: Add SDP_IS_TEXT_STR() macro for SDP_TEXT_STR* checking

[PATCH BlueZ 1/5] lib: Add SDP_IS_TEXT_STR() macro for SDP_TEXT_STR* checking

[Anderson Lizardo]

This new macro avoids constructs like "if (d->dtd < SDP_TEXT_STR8 ||
d->dtd > SDP_TEXT_STR32)" which are harder to read.
---
 lib/sdp.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/sdp.h b/lib/sdp.h
index 4448805..42681a2 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -440,6 +440,8 @@ typedef struct {
 
 #define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || (x) ==SDP_UUID128)
 #define SDP_IS_SEQ(x)  ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
+#define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
+							(x) == SDP_TEXT_STR32)
 
 typedef struct _sdp_list sdp_list_t;
 struct _sdp_list {
-- 
1.7.9.5

[PATCH BlueZ 2/5] lib: Trivial whitespace and line wrapping fix

[Anderson Lizardo]

---
 lib/sdp.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/sdp.h b/lib/sdp.h
index 42681a2..a81e857 100644
--- a/lib/sdp.h
+++ b/lib/sdp.h
@@ -438,7 +438,8 @@ typedef struct {
 	} value;
 } uuid_t;
 
-#define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || (x) ==SDP_UUID128)
+#define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || \
+							(x) == SDP_UUID128)
 #define SDP_IS_SEQ(x)  ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
 #define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
 							(x) == SDP_TEXT_STR32)
-- 
1.7.9.5

[PATCH v2 BlueZ 3/5] input: Validate SDP HIDDescriptorList subattributes

[Anderson Lizardo]

It should not be assumed that remote SDP attributes are in a compliant
format. This fixes a couple of invalid pointer access on invalid data.
---
 profiles/input/device.c |   60 ++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 46 insertions(+), 14 deletions(-)

diff --git a/profiles/input/device.c b/profiles/input/device.c
index eaf5681..1da9d99 100644
--- a/profiles/input/device.c
+++ b/profiles/input/device.c
@@ -222,6 +222,49 @@ static int create_hid_dev_name(sdp_record_t *rec, struct hidp_connadd_req *req)
 	return 0;
 }
 
+/* See HID profile specification v1.0, "7.11.6 HIDDescriptorList" for details
+ * on the attribute format. */
+static int extract_hid_desc_data(sdp_record_t *rec,
+						struct hidp_connadd_req *req)
+{
+	sdp_data_t *d;
+
+	d = sdp_data_get(rec, SDP_ATTR_HID_DESCRIPTOR_LIST);
+	if (!d)
+		goto invalid_desc;
+
+	if (!SDP_IS_SEQ(d->dtd))
+		goto invalid_desc;
+
+	/* First HIDDescriptor */
+	d = d->val.dataseq;
+	if (!SDP_IS_SEQ(d->dtd))
+		goto invalid_desc;
+
+	/* ClassDescriptorType */
+	d = d->val.dataseq;
+	if (d->dtd != SDP_UINT8)
+		goto invalid_desc;
+
+	/* ClassDescriptorData */
+	d = d->next;
+	if (!d || !SDP_IS_TEXT_STR(d->dtd))
+		goto invalid_desc;
+
+	req->rd_data = g_try_malloc0(d->unitSize);
+	if (req->rd_data) {
+		memcpy(req->rd_data, d->val.str, d->unitSize);
+		req->rd_size = d->unitSize;
+		epox_endian_quirk(req->rd_data, req->rd_size);
+	}
+
+	return 0;
+
+invalid_desc:
+	error("Missing or invalid HIDDescriptorList SDP attribute");
+	return -EINVAL;
+}
+
 static int extract_hid_record(sdp_record_t *rec, struct hidp_connadd_req *req)
 {
 	sdp_data_t *pdlist;
@@ -251,20 +294,9 @@ static int extract_hid_record(sdp_record_t *rec, struct hidp_connadd_req *req)
 	if (attr_val)
 		req->flags |= (1 << HIDP_BOOT_PROTOCOL_MODE);
 
-	pdlist = sdp_data_get(rec, SDP_ATTR_HID_DESCRIPTOR_LIST);
-	if (pdlist) {
-		pdlist = pdlist->val.dataseq;
-		pdlist = pdlist->val.dataseq;
-		pdlist = pdlist->next;
-
-		req->rd_data = g_try_malloc0(pdlist->unitSize);
-		if (req->rd_data) {
-			memcpy(req->rd_data, (unsigned char *) pdlist->val.str,
-								pdlist->unitSize);
-			req->rd_size = pdlist->unitSize;
-			epox_endian_quirk(req->rd_data, req->rd_size);
-		}
-	}
+	err = extract_hid_desc_data(rec, req);
+	if (err < 0)
+		return err;
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH BlueZ 4/5] lib: Use SDP_IS_TEXT_STR()/SDP_IS_SEQ() where possible

[Anderson Lizardo]

---
 lib/sdp.c |   12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 2dac6c7..ca474cd 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -570,7 +570,7 @@ static void extract_svclass_uuid(sdp_data_t *data, uuid_t *uuid)
 {
 	sdp_data_t *d;
 
-	if (!data || data->dtd < SDP_SEQ8 || data->dtd > SDP_SEQ32)
+	if (!data || !SDP_IS_SEQ(data->dtd))
 		return;
 
 	d = data->val.dataseq;
@@ -1912,7 +1912,7 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
 	sdp_data_t *sdpdata = sdp_data_get(rec, attr);
 
 	*seqp = NULL;
-	if (sdpdata && sdpdata->dtd >= SDP_SEQ8 && sdpdata->dtd <= SDP_SEQ32) {
+	if (sdpdata && SDP_IS_SEQ(sdpdata->dtd)) {
 		sdp_data_t *d;
 		for (d = sdpdata->val.dataseq; d; d = d->next) {
 			uuid_t *u;
@@ -2128,9 +2128,7 @@ int sdp_get_string_attr(const sdp_record_t *rec, uint16_t attrid, char *value,
 	sdp_data_t *sdpdata = sdp_data_get(rec, attrid);
 	if (sdpdata)
 		/* Verify that it is what the caller expects */
-		if (sdpdata->dtd == SDP_TEXT_STR8 ||
-				sdpdata->dtd == SDP_TEXT_STR16 ||
-				sdpdata->dtd == SDP_TEXT_STR32)
+		if (SDP_IS_TEXT_STR(sdpdata->dtd))
 			if ((int) strlen(sdpdata->val.str) < valuelen) {
 				strcpy(value, sdpdata->val.str);
 				return 0;
@@ -4744,7 +4742,7 @@ int sdp_get_supp_feat(const sdp_record_t *rec, sdp_list_t **seqp)
 
 	sdpdata = sdp_data_get(rec, SDP_ATTR_SUPPORTED_FEATURES_LIST);
 
-	if (!sdpdata || sdpdata->dtd < SDP_SEQ8 || sdpdata->dtd > SDP_SEQ32)
+	if (!sdpdata || !SDP_IS_SEQ(sdpdata->dtd))
 		return sdp_get_uuidseq_attr(rec,
 					SDP_ATTR_SUPPORTED_FEATURES_LIST, seqp);
 
@@ -4752,7 +4750,7 @@ int sdp_get_supp_feat(const sdp_record_t *rec, sdp_list_t **seqp)
 		sdp_data_t *dd;
 		sdp_list_t *subseq;
 
-		if (d->dtd < SDP_SEQ8 || d->dtd > SDP_SEQ32)
+		if (!SDP_IS_SEQ(d->dtd))
 			goto fail;
 
 		subseq = NULL;
-- 
1.7.9.5

[PATCH BlueZ 5/5] health: Use SDP_IS_TEXT_STR()/SDP_IS_SEQ() where possible

[Anderson Lizardo]

---
 profiles/health/hdp_util.c |   18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 5799ae5..5f81806 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -767,15 +767,13 @@ static gboolean get_mdep_from_rec(const sdp_record_t *rec, uint8_t role,
 		return TRUE;
 
 	list = sdp_data_get(rec, SDP_ATTR_SUPPORTED_FEATURES_LIST);
-	if (list == NULL || (list->dtd != SDP_SEQ8 && list->dtd != SDP_SEQ16 &&
-							list->dtd != SDP_SEQ32))
+	if (list == NULL || !SDP_IS_SEQ(list->dtd))
 		return FALSE;
 
 	for (feat = list->val.dataseq; feat; feat = feat->next) {
 		sdp_data_t *data_type, *mdepid, *role_t, *desc_t;
 
-		if (feat->dtd != SDP_SEQ8 && feat->dtd != SDP_SEQ16 &&
-						feat->dtd != SDP_SEQ32)
+		if (!SDP_IS_SEQ(feat->dtd))
 			continue;
 
 		mdepid = feat->val.dataseq;
@@ -803,10 +801,8 @@ static gboolean get_mdep_from_rec(const sdp_record_t *rec, uint8_t role,
 		if (mdep != NULL)
 			*mdep = mdepid->val.uint8;
 
-		if (desc != NULL  && desc_t != NULL  &&
-					(desc_t->dtd == SDP_TEXT_STR8 ||
-					desc_t->dtd == SDP_TEXT_STR16  ||
-					desc_t->dtd == SDP_TEXT_STR32))
+		if (desc != NULL && desc_t != NULL &&
+						SDP_IS_TEXT_STR(desc_t->dtd))
 			*desc = g_strdup(desc_t->val.str);
 
 		return TRUE;
@@ -887,8 +883,7 @@ static gboolean get_prot_desc_entry(sdp_data_t *entry, int type, guint16 *val)
 	sdp_data_t *iter;
 	int proto;
 
-	if (entry == NULL || (entry->dtd != SDP_SEQ8 &&
-			entry->dtd != SDP_SEQ16 && entry->dtd != SDP_SEQ32))
+	if (entry == NULL || !SDP_IS_SEQ(entry->dtd))
 		return FALSE;
 
 	iter = entry->val.dataseq;
@@ -920,8 +915,7 @@ static gboolean hdp_get_prot_desc_list(const sdp_record_t *rec, guint16 *psm,
 		return TRUE;
 
 	pdl = sdp_data_get(rec, SDP_ATTR_PROTO_DESC_LIST);
-	if (pdl == NULL || (pdl->dtd != SDP_SEQ8 && pdl->dtd != SDP_SEQ16 &&
-							pdl->dtd != SDP_SEQ32))
+	if (pdl == NULL || !SDP_IS_SEQ(pdl->dtd))
 		return FALSE;
 
 	p0 = pdl->val.dataseq;
-- 
1.7.9.5

Re: [PATCH BlueZ 1/5] lib: Add SDP_IS_TEXT_STR() macro for SDP_TEXT_STR* checking

[Johan Hedberg]

Hi Lizardo,

On Mon, Jan 07, 2013, Anderson Lizardo wrote:
> This new macro avoids constructs like "if (d->dtd < SDP_TEXT_STR8 ||
> d->dtd > SDP_TEXT_STR32)" which are harder to read.
> ---
>  lib/sdp.h |    2 ++
>  1 file changed, 2 insertions(+)

All patches in this set have been applied. Thanks.

Johan

Re: [PATCH BlueZ 1/5] lib: Add SDP_IS_TEXT_STR() macro for SDP_TEXT_STR* checking

[Marcel Holtmann]

Hi Anderson,

> This new macro avoids constructs like "if (d->dtd < SDP_TEXT_STR8 ||
> d->dtd > SDP_TEXT_STR32)" which are harder to read.
> ---
>  lib/sdp.h |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/sdp.h b/lib/sdp.h
> index 4448805..42681a2 100644
> --- a/lib/sdp.h
> +++ b/lib/sdp.h
> @@ -440,6 +440,8 @@ typedef struct {
>  
>  #define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || (x) ==SDP_UUID128)
>  #define SDP_IS_SEQ(x)  ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
> +#define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
> +							(x) == SDP_TEXT_STR32)

can someone please explain to me why we are extending the library. I
thought I made it clear that it is mostly end of life and we are not
adding new features. However some people keep adding stuff.

Regards

Marcel

Re: [PATCH BlueZ 1/5] lib: Add SDP_IS_TEXT_STR() macro for SDP_TEXT_STR* checking

[Anderson Lizardo]

Hi Marcel,

On Mon, Jan 7, 2013 at 11:25 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>>  #define SDP_IS_UUID(x) ((x) == SDP_UUID16 || (x) == SDP_UUID32 || (x) ==SDP_UUID128)
>>  #define SDP_IS_SEQ(x)  ((x) == SDP_SEQ8 || (x) == SDP_SEQ16 || (x) == SDP_SEQ32)
>> +#define SDP_IS_TEXT_STR(x) ((x) == SDP_TEXT_STR8 || (x) == SDP_TEXT_STR16 || \
>> +                                                     (x) == SDP_TEXT_STR32)
>
> can someone please explain to me why we are extending the library. I
> thought I made it clear that it is mostly end of life and we are not
> adding new features. However some people keep adding stuff.

This macro was added simply to be used for the next patches which
simplify/refactor core code. The fact the header is public API was not
the main reason to add it here.

Unless you suggest adding new stuff to another, internal-only header?
If so, please provide some example how to extend SDP library code that
is used by internal BlueZ code.

Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil