2012-10-10 23:34:58

by Vinicius Costa Gomes

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

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



2012-10-11 06:57:05

by Johan Hedberg

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

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

2012-10-10 23:35:03

by Vinicius Costa Gomes

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

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


2012-10-10 23:35:02

by Vinicius Costa Gomes

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

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


2012-10-10 23:35:01

by Vinicius Costa Gomes

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

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


2012-10-10 23:35:00

by Vinicius Costa Gomes

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

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


2012-10-10 23:34:59

by Vinicius Costa Gomes

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

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