[PATCH BlueZ 1/6] gattrib: Fix ignoring the error message when write fails

[PATCH BlueZ 1/6] gattrib: Fix ignoring the error message when write fails

[Vinicius Costa Gomes]

If an error happens during writing to the socket, we should complain
that it failed.
---
 attrib/gattrib.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/attrib/gattrib.c b/attrib/gattrib.c
index 6f6942f..0806101 100644
--- a/attrib/gattrib.c
+++ b/attrib/gattrib.c
@@ -313,8 +313,14 @@ static gboolean can_write_data(GIOChannel *io, GIOCondition cond,
 
 	iostat = g_io_channel_write_chars(io, (gchar *) cmd->pdu, cmd->len,
 								&len, &gerr);
-	if (iostat != G_IO_STATUS_NORMAL)
+	if (iostat != G_IO_STATUS_NORMAL) {
+		if (gerr) {
+			error("%s", gerr->message);
+			g_error_free(gerr);
+		}
+
 		return FALSE;
+	}
 
 	if (cmd->expected == 0) {
 		g_queue_pop_head(queue);
-- 
1.7.12.3

[PATCH BlueZ 2/6] attrib: Remove all the usages of ATT_MAX_MTU

[Vinicius Costa Gomes]

This "define" was bogus for two reasons: 1. There's no concept
of maximum MTU in the ATT level; 2. It was used as a maximum attribute
value length.
---
 attrib/gatttool.c       |  2 +-
 attrib/interactive.c    |  2 +-
 profiles/alert/server.c | 11 ++++++-----
 src/attrib-server.c     |  3 ++-
 4 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/attrib/gatttool.c b/attrib/gatttool.c
index 16cce0c..252064d 100644
--- a/attrib/gatttool.c
+++ b/attrib/gatttool.c
@@ -227,7 +227,7 @@ static gboolean characteristics(gpointer user_data)
 static void char_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
-	uint8_t value[ATT_MAX_MTU];
+	uint8_t value[plen];
 	ssize_t vlen;
 	int i;
 
diff --git a/attrib/interactive.c b/attrib/interactive.c
index b41a7bb..716e675 100644
--- a/attrib/interactive.c
+++ b/attrib/interactive.c
@@ -309,7 +309,7 @@ static void char_desc_cb(guint8 status, const guint8 *pdu, guint16 plen,
 static void char_read_cb(guint8 status, const guint8 *pdu, guint16 plen,
 							gpointer user_data)
 {
-	uint8_t value[ATT_MAX_MTU];
+	uint8_t value[plen];
 	ssize_t vlen;
 	int i;
 
diff --git a/profiles/alert/server.c b/profiles/alert/server.c
index 77de704..761a24b 100644
--- a/profiles/alert/server.c
+++ b/profiles/alert/server.c
@@ -353,24 +353,25 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	struct notify_data *nd = cb->notify_data;
 	enum notify_type type = nd->type;
 	struct alert_adapter *al_adapter = nd->al_adapter;
-	uint8_t pdu[ATT_MAX_MTU];
-	size_t len = 0;
+	size_t len;
+	uint8_t *pdu = g_attrib_get_buffer(attrib, &len);
+
 
 	switch (type) {
 	case NOTIFY_RINGER_SETTING:
 		len = enc_notification(al_adapter->hnd_value[type],
 				&ringer_setting, sizeof(ringer_setting),
-				pdu, sizeof(pdu));
+				pdu, len);
 		break;
 	case NOTIFY_ALERT_STATUS:
 		len = enc_notification(al_adapter->hnd_value[type],
 				&alert_status, sizeof(alert_status),
-				pdu, sizeof(pdu));
+				pdu, len);
 		break;
 	case NOTIFY_NEW_ALERT:
 	case NOTIFY_UNREAD_ALERT:
 		len = enc_notification(al_adapter->hnd_value[type],
-					nd->value, nd->len, pdu, sizeof(pdu));
+					nd->value, nd->len, pdu, len);
 		break;
 	default:
 		DBG("Unknown type, could not send notification");
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 76a32af..ec4ecc3 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -907,11 +907,12 @@ static void channel_handler(const uint8_t *ipdu, uint16_t len,
 							gpointer user_data)
 {
 	struct gatt_channel *channel = user_data;
-	uint8_t opdu[channel->mtu], value[ATT_MAX_MTU];
+	uint8_t opdu[channel->mtu];
 	uint16_t length, start, end, mtu, offset;
 	bt_uuid_t uuid;
 	uint8_t status = 0;
 	size_t vlen;
+	uint8_t *value = g_attrib_get_buffer(channel->attrib, &vlen);
 
 	DBG("op 0x%02x", ipdu[0]);
 
-- 
1.7.12.3

[PATCH BlueZ 3/6] att: Replace ATT_MAX_MTU with ATT_MAX_VALUE_LEN

[Vinicius Costa Gomes]

ATT has the concept that an attribute value has a maximum length and we
weren't keeping track of this.
---
 attrib/att.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/attrib/att.h b/attrib/att.h
index a563255..ebdb98e 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -85,7 +85,7 @@
 #define ATT_CHAR_PROPER_AUTH			0x40
 #define ATT_CHAR_PROPER_EXT_PROPER		0x80
 
-#define ATT_MAX_MTU				256
+#define ATT_MAX_VALUE_LEN			512
 #define ATT_DEFAULT_L2CAP_MTU			48
 #define ATT_DEFAULT_LE_MTU			23
 
-- 
1.7.12.3

[PATCH BlueZ 4/6] att: Fix sending pdu's with invalid data

[Vinicius Costa Gomes]

When encoding an att_data_list we need to make sure that each element
lenght of the data list will not exceed 255, because that information
will be encoded as a octet later.
---
 attrib/att.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/attrib/att.c b/attrib/att.c
index fc510f4..f262bb6 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -101,6 +101,9 @@ struct att_data_list *att_data_list_alloc(uint16_t num, uint16_t len)
 	struct att_data_list *list;
 	int i;
 
+	if (len > UINT8_MAX)
+		return NULL;
+
 	list = g_new0(struct att_data_list, 1);
 	list->len = len;
 	list->num = num;
-- 
1.7.12.3

[PATCH BlueZ 5/6] attrib: Fix not checking if att_data_list_alloc fails

[Vinicius Costa Gomes]

Now that this function may fail in more usual situations (invalid
input), we have to check its return value.
---
 attrib/att.c        | 6 ++++++
 src/attrib-server.c | 9 +++++++++
 2 files changed, 15 insertions(+)

diff --git a/attrib/att.c b/attrib/att.c
index f262bb6..0ed4178 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -211,6 +211,8 @@ struct att_data_list *dec_read_by_grp_resp(const uint8_t *pdu, size_t len)
 	elen = pdu[1];
 	num = (len - 2) / elen;
 	list = att_data_list_alloc(num, elen);
+	if (list == NULL)
+		return NULL;
 
 	ptr = &pdu[2];
 
@@ -441,6 +443,8 @@ struct att_data_list *dec_read_by_type_resp(const uint8_t *pdu, size_t len)
 	elen = pdu[1];
 	num = (len - 2) / elen;
 	list = att_data_list_alloc(num, elen);
+	if (list == NULL)
+		return NULL;
 
 	ptr = &pdu[2];
 
@@ -825,6 +829,8 @@ struct att_data_list *dec_find_info_resp(const uint8_t *pdu, size_t len,
 	ptr = (void *) &pdu[2];
 
 	list = att_data_list_alloc(num, elen);
+	if (list == NULL)
+		return NULL;
 
 	for (i = 0; i < num; i++) {
 		memcpy(list->data[i], ptr, list->len);
diff --git a/src/attrib-server.c b/src/attrib-server.c
index ec4ecc3..7117fbe 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -490,6 +490,9 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
 	length = g_slist_length(groups);
 
 	adl = att_data_list_alloc(length, last_size + 4);
+	if (adl == NULL)
+		return enc_error_resp(ATT_OP_READ_BY_GROUP_REQ, start,
+					ATT_ECODE_UNLIKELY, pdu, len);
 
 	for (i = 0, l = groups; l; l = l->next, i++) {
 		uint8_t *value;
@@ -574,6 +577,9 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
 	length += 2;
 
 	adl = att_data_list_alloc(num, length);
+	if (adl == NULL)
+		return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ, start,
+					ATT_ECODE_UNLIKELY, pdu, len);
 
 	for (i = 0, l = types; l; i++, l = l->next) {
 		uint8_t *value;
@@ -649,6 +655,9 @@ static uint16_t find_info(struct gatt_channel *channel, uint16_t start,
 	}
 
 	adl = att_data_list_alloc(num, length + 2);
+	if (adl == NULL)
+		return enc_error_resp(ATT_OP_FIND_INFO_REQ, start,
+					ATT_ECODE_UNLIKELY, pdu, len);
 
 	for (i = 0, l = info; l; i++, l = l->next) {
 		uint8_t *value;
-- 
1.7.12.3

[PATCH BlueZ 6/6] gas: Only do the Exchange MTU procedure over LE links

[Vinicius Costa Gomes]

The Exchange MTU procedure should only be performed over LE links,
we are using the check of the Channel ID used to verify this.
---
 profiles/gatt/gas.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/profiles/gatt/gas.c b/profiles/gatt/gas.c
index 74ca9ce..f873121 100644
--- a/profiles/gatt/gas.c
+++ b/profiles/gatt/gas.c
@@ -326,7 +326,8 @@ static void attio_connected_cb(GAttrib *attrib, gpointer user_data)
 	io = g_attrib_get_channel(attrib);
 
 	if (bt_io_get(io, &gerr, BT_IO_OPT_IMTU, &imtu,
-				BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID)) {
+				BT_IO_OPT_CID, &cid, BT_IO_OPT_INVALID) &&
+							cid == ATT_CID) {
 		gatt_exchange_mtu(gas->attrib, imtu, exchange_mtu_cb, gas);
 		gas->mtu = imtu;
 		DBG("MTU Exchange: Requesting %d", imtu);
-- 
1.7.12.3

Re: [PATCH BlueZ 1/6] gattrib: Fix ignoring the error message when write fails

[Johan Hedberg]

Hi Vinicius,

On Wed, Oct 10, 2012, Vinicius Costa Gomes wrote:
> If an error happens during writing to the socket, we should complain
> that it failed.
> ---
>  attrib/gattrib.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

All patches in this set have been applied. Thanks.

Johan