* [Bluez PATCH v1] shared/gatt-server: Fix read multiple charc values
@ 2020-05-06 11:44 Archie Pusaka
2020-05-06 12:09 ` [Bluez,v1] " bluez.test.bot
2020-05-06 17:28 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
0 siblings, 2 replies; 4+ messages in thread
From: Archie Pusaka @ 2020-05-06 11:44 UTC (permalink / raw)
To: linux-bluetooth, Luiz Augusto von Dentz; +Cc: Archie Pusaka
From: Archie Pusaka <apusaka@chromium.org>
According to bluetooth spec Ver 5.2, Vol 3, Part G, 4.8.4, An
ATT_ERROR_RSP PDU shall be sent by the server in response to the
ATT_READ_MULTIPLE_RSP PDU if insufficient authentication,
insufficient authorization, insufficient encryption key size, or
insufficient encryption is used by the client, or if a read operation
is not permitted on any of the Characteristic Values.
Currently if the size of the response grows larger than the MTU size,
BlueZ does an early return and not check the permission for the rest
of the characteristics. This patch forces BlueZ to check for possible
errors even though we already reach MTU size.
This patch also moves the read permission check for read multiple
characteristics so it is done before actually retrieving the
characteristics.
---
src/shared/gatt-server.c | 88 ++++++++++++++++++++--------------------
1 file changed, 45 insertions(+), 43 deletions(-)
diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
index 4e07398d2..e937d80a8 100644
--- a/src/shared/gatt-server.c
+++ b/src/shared/gatt-server.c
@@ -1057,33 +1057,23 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
uint16_t length;
if (err != 0) {
- bt_att_chan_send_error_rsp(data->chan, data->opcode, handle,
- err);
- read_mult_data_free(data);
- return;
- }
-
- ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
- BT_ATT_PERM_READ_AUTHEN |
- BT_ATT_PERM_READ_ENCRYPT);
- if (ecode) {
- bt_att_chan_send_error_rsp(data->chan, data->opcode, handle,
- ecode);
- read_mult_data_free(data);
- return;
+ ecode = err;
+ goto error;
}
length = data->opcode == BT_ATT_OP_READ_MULT_VL_REQ ?
- MIN(len, data->mtu - data->length - 3) :
+ MIN(len, MAX(0, data->mtu - data->length - 3)) :
MIN(len, data->mtu - data->length - 1);
if (data->opcode == BT_ATT_OP_READ_MULT_VL_REQ) {
/* The Length Value Tuple List may be truncated within the first
* two octets of a tuple due to the size limits of the current
- * ATT_MTU.
+ * ATT_MTU, but the first two octets cannot be separated.
*/
- put_le16(len, data->rsp_data + data->length);
- data->length += 2;
+ if (data->mtu - data->length >= 3) {
+ put_le16(len, data->rsp_data + data->length);
+ data->length += 2;
+ }
}
memcpy(data->rsp_data + data->length, value, length);
@@ -1091,45 +1081,46 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
data->cur_handle++;
- len = data->opcode == BT_ATT_OP_READ_MULT_VL_REQ ?
- data->mtu - data->length - 3 : data->mtu - data->length - 1;
-
- if (!len || (data->cur_handle == data->num_handles)) {
+ if (data->cur_handle == data->num_handles) {
bt_att_chan_send_rsp(data->chan, data->opcode + 1,
data->rsp_data, data->length);
read_mult_data_free(data);
return;
}
+ handle = data->handles[data->cur_handle];
+
util_debug(data->server->debug_callback, data->server->debug_data,
"%s Req - #%zu of %zu: 0x%04x",
data->opcode == BT_ATT_OP_READ_MULT_REQ ?
"Read Multiple" :
"Read Multiple Variable Length",
data->cur_handle + 1, data->num_handles,
- data->handles[data->cur_handle]);
+ handle);
- next_attr = gatt_db_get_attribute(data->server->db,
- data->handles[data->cur_handle]);
+ next_attr = gatt_db_get_attribute(data->server->db, handle);
if (!next_attr) {
- bt_att_chan_send_error_rsp(data->chan,
- BT_ATT_OP_READ_MULT_REQ,
- data->handles[data->cur_handle],
- BT_ATT_ERROR_INVALID_HANDLE);
- read_mult_data_free(data);
- return;
+ ecode = BT_ATT_ERROR_INVALID_HANDLE;
+ goto error;
}
- if (!gatt_db_attribute_read(next_attr, 0, BT_ATT_OP_READ_MULT_REQ,
+ ecode = check_permissions(data->server, next_attr, BT_ATT_PERM_READ |
+ BT_ATT_PERM_READ_AUTHEN |
+ BT_ATT_PERM_READ_ENCRYPT);
+ if (ecode)
+ goto error;
+
+ if (gatt_db_attribute_read(next_attr, 0, data->opcode,
data->server->att,
- read_multiple_complete_cb, data)) {
- bt_att_chan_send_error_rsp(data->chan,
- BT_ATT_OP_READ_MULT_REQ,
- data->handles[data->cur_handle],
- BT_ATT_ERROR_UNLIKELY);
- read_mult_data_free(data);
- }
+ read_multiple_complete_cb, data))
+ return;
+
+ ecode = BT_ATT_ERROR_UNLIKELY;
+
+error:
+ bt_att_chan_send_error_rsp(data->chan, data->opcode, handle, ecode);
+ read_mult_data_free(data);
}
static struct read_mult_data *read_mult_data_new(struct bt_gatt_server *server,
@@ -1161,8 +1152,9 @@ static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
struct bt_gatt_server *server = user_data;
struct gatt_db_attribute *attr;
struct read_mult_data *data = NULL;
- uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
+ uint8_t ecode;
size_t i = 0;
+ uint16_t handle = 0;
if (length < 4) {
ecode = BT_ATT_ERROR_INVALID_PDU;
@@ -1176,28 +1168,38 @@ static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
for (i = 0; i < data->num_handles; i++)
data->handles[i] = get_le16(pdu + i * 2);
+ handle = data->handles[0];
+
util_debug(server->debug_callback, server->debug_data,
"%s Req - %zu handles, 1st: 0x%04x",
data->opcode == BT_ATT_OP_READ_MULT_REQ ?
"Read Multiple" : "Read Multiple Variable Length",
- data->num_handles, data->handles[0]);
+ data->num_handles, handle);
- attr = gatt_db_get_attribute(server->db, data->handles[0]);
+ attr = gatt_db_get_attribute(server->db, handle);
if (!attr) {
ecode = BT_ATT_ERROR_INVALID_HANDLE;
goto error;
}
+ ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
+ BT_ATT_PERM_READ_AUTHEN |
+ BT_ATT_PERM_READ_ENCRYPT);
+ if (ecode)
+ goto error;
+
if (gatt_db_attribute_read(attr, 0, opcode, server->att,
read_multiple_complete_cb, data))
return;
+ ecode = BT_ATT_ERROR_UNLIKELY;
+
error:
if (data)
read_mult_data_free(data);
- bt_att_chan_send_error_rsp(chan, opcode, 0, ecode);
+ bt_att_chan_send_error_rsp(chan, opcode, handle, ecode);
}
static bool append_prep_data(struct prep_write_data *prep_data, uint16_t handle,
--
2.26.2.526.g744177e7f7-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: [Bluez,v1] shared/gatt-server: Fix read multiple charc values
2020-05-06 11:44 [Bluez PATCH v1] shared/gatt-server: Fix read multiple charc values Archie Pusaka
@ 2020-05-06 12:09 ` bluez.test.bot
2020-05-06 17:28 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
1 sibling, 0 replies; 4+ messages in thread
From: bluez.test.bot @ 2020-05-06 12:09 UTC (permalink / raw)
To: linux-bluetooth, apusaka
[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
While we are preparing for reviewing the patches, we found the following
issue/warning.
Test Result:
Checkbuild Failed
Patch Series:
[Bluez,v1] shared/gatt-server: Fix read multiple charc values
Outputs:
ar: `u' modifier ignored since `D' is the default (see `U')
src/shared/gatt-server.c: In function ‘read_multiple_complete_cb’:
src/shared/gatt-server.c:42:24: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
42 | #define MAX(a, b) ((a) > (b) ? (a) : (b))
| ^
src/shared/gatt-server.c:46:27: note: in definition of macro ‘MIN’
46 | #define MIN(a, b) ((a) < (b) ? (a) : (b))
| ^
src/shared/gatt-server.c:1065:13: note: in expansion of macro ‘MAX’
1065 | MIN(len, MAX(0, data->mtu - data->length - 3)) :
| ^~~
src/shared/gatt-server.c:42:24: error: comparison of unsigned expression < 0 is always false [-Werror=type-limits]
42 | #define MAX(a, b) ((a) > (b) ? (a) : (b))
| ^
src/shared/gatt-server.c:46:39: note: in definition of macro ‘MIN’
46 | #define MIN(a, b) ((a) < (b) ? (a) : (b))
| ^
src/shared/gatt-server.c:1065:13: note: in expansion of macro ‘MAX’
1065 | MIN(len, MAX(0, data->mtu - data->length - 3)) :
| ^~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:6807: src/shared/gatt-server.lo] Error 1
make: *** [Makefile:4010: all] Error 2
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Bluez PATCH v1] shared/gatt-server: Fix read multiple charc values
2020-05-06 11:44 [Bluez PATCH v1] shared/gatt-server: Fix read multiple charc values Archie Pusaka
2020-05-06 12:09 ` [Bluez,v1] " bluez.test.bot
@ 2020-05-06 17:28 ` Luiz Augusto von Dentz
2020-05-07 2:02 ` Archie Pusaka
1 sibling, 1 reply; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2020-05-06 17:28 UTC (permalink / raw)
To: Archie Pusaka; +Cc: linux-bluetooth, Archie Pusaka
Hi Archie,
On Wed, May 6, 2020 at 4:45 AM Archie Pusaka <apusaka@google.com> wrote:
>
> From: Archie Pusaka <apusaka@chromium.org>
>
> According to bluetooth spec Ver 5.2, Vol 3, Part G, 4.8.4, An
> ATT_ERROR_RSP PDU shall be sent by the server in response to the
> ATT_READ_MULTIPLE_RSP PDU if insufficient authentication,
> insufficient authorization, insufficient encryption key size, or
> insufficient encryption is used by the client, or if a read operation
> is not permitted on any of the Characteristic Values.
>
> Currently if the size of the response grows larger than the MTU size,
> BlueZ does an early return and not check the permission for the rest
> of the characteristics. This patch forces BlueZ to check for possible
> errors even though we already reach MTU size.
>
> This patch also moves the read permission check for read multiple
> characteristics so it is done before actually retrieving the
> characteristics.
Is there a test failing because of this? If there is not I would have
thought this is actually a prefered behavior since the subsequent read
would cause an error at least the client would be able to the data it
could read in the beginning otherwise this will be imposing the
database to read all the handles discarding the data that don't fit
just to propagate the errors before the data can actually be read.
> ---
>
> src/shared/gatt-server.c | 88 ++++++++++++++++++++--------------------
> 1 file changed, 45 insertions(+), 43 deletions(-)
>
> diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> index 4e07398d2..e937d80a8 100644
> --- a/src/shared/gatt-server.c
> +++ b/src/shared/gatt-server.c
> @@ -1057,33 +1057,23 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
> uint16_t length;
>
> if (err != 0) {
> - bt_att_chan_send_error_rsp(data->chan, data->opcode, handle,
> - err);
> - read_mult_data_free(data);
> - return;
> - }
> -
> - ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
> - BT_ATT_PERM_READ_AUTHEN |
> - BT_ATT_PERM_READ_ENCRYPT);
> - if (ecode) {
> - bt_att_chan_send_error_rsp(data->chan, data->opcode, handle,
> - ecode);
> - read_mult_data_free(data);
> - return;
> + ecode = err;
> + goto error;
> }
>
> length = data->opcode == BT_ATT_OP_READ_MULT_VL_REQ ?
> - MIN(len, data->mtu - data->length - 3) :
> + MIN(len, MAX(0, data->mtu - data->length - 3)) :
> MIN(len, data->mtu - data->length - 1);
>
> if (data->opcode == BT_ATT_OP_READ_MULT_VL_REQ) {
> /* The Length Value Tuple List may be truncated within the first
> * two octets of a tuple due to the size limits of the current
> - * ATT_MTU.
> + * ATT_MTU, but the first two octets cannot be separated.
> */
> - put_le16(len, data->rsp_data + data->length);
> - data->length += 2;
> + if (data->mtu - data->length >= 3) {
> + put_le16(len, data->rsp_data + data->length);
> + data->length += 2;
> + }
> }
>
> memcpy(data->rsp_data + data->length, value, length);
> @@ -1091,45 +1081,46 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
>
> data->cur_handle++;
>
> - len = data->opcode == BT_ATT_OP_READ_MULT_VL_REQ ?
> - data->mtu - data->length - 3 : data->mtu - data->length - 1;
> -
> - if (!len || (data->cur_handle == data->num_handles)) {
> + if (data->cur_handle == data->num_handles) {
> bt_att_chan_send_rsp(data->chan, data->opcode + 1,
> data->rsp_data, data->length);
> read_mult_data_free(data);
> return;
> }
>
> + handle = data->handles[data->cur_handle];
> +
> util_debug(data->server->debug_callback, data->server->debug_data,
> "%s Req - #%zu of %zu: 0x%04x",
> data->opcode == BT_ATT_OP_READ_MULT_REQ ?
> "Read Multiple" :
> "Read Multiple Variable Length",
> data->cur_handle + 1, data->num_handles,
> - data->handles[data->cur_handle]);
> + handle);
>
> - next_attr = gatt_db_get_attribute(data->server->db,
> - data->handles[data->cur_handle]);
> + next_attr = gatt_db_get_attribute(data->server->db, handle);
>
> if (!next_attr) {
> - bt_att_chan_send_error_rsp(data->chan,
> - BT_ATT_OP_READ_MULT_REQ,
> - data->handles[data->cur_handle],
> - BT_ATT_ERROR_INVALID_HANDLE);
> - read_mult_data_free(data);
> - return;
> + ecode = BT_ATT_ERROR_INVALID_HANDLE;
> + goto error;
> }
>
> - if (!gatt_db_attribute_read(next_attr, 0, BT_ATT_OP_READ_MULT_REQ,
> + ecode = check_permissions(data->server, next_attr, BT_ATT_PERM_READ |
> + BT_ATT_PERM_READ_AUTHEN |
> + BT_ATT_PERM_READ_ENCRYPT);
> + if (ecode)
> + goto error;
> +
> + if (gatt_db_attribute_read(next_attr, 0, data->opcode,
> data->server->att,
> - read_multiple_complete_cb, data)) {
> - bt_att_chan_send_error_rsp(data->chan,
> - BT_ATT_OP_READ_MULT_REQ,
> - data->handles[data->cur_handle],
> - BT_ATT_ERROR_UNLIKELY);
> - read_mult_data_free(data);
> - }
> + read_multiple_complete_cb, data))
> + return;
> +
> + ecode = BT_ATT_ERROR_UNLIKELY;
> +
> +error:
> + bt_att_chan_send_error_rsp(data->chan, data->opcode, handle, ecode);
> + read_mult_data_free(data);
> }
>
> static struct read_mult_data *read_mult_data_new(struct bt_gatt_server *server,
> @@ -1161,8 +1152,9 @@ static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
> struct bt_gatt_server *server = user_data;
> struct gatt_db_attribute *attr;
> struct read_mult_data *data = NULL;
> - uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
> + uint8_t ecode;
> size_t i = 0;
> + uint16_t handle = 0;
>
> if (length < 4) {
> ecode = BT_ATT_ERROR_INVALID_PDU;
> @@ -1176,28 +1168,38 @@ static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
> for (i = 0; i < data->num_handles; i++)
> data->handles[i] = get_le16(pdu + i * 2);
>
> + handle = data->handles[0];
> +
> util_debug(server->debug_callback, server->debug_data,
> "%s Req - %zu handles, 1st: 0x%04x",
> data->opcode == BT_ATT_OP_READ_MULT_REQ ?
> "Read Multiple" : "Read Multiple Variable Length",
> - data->num_handles, data->handles[0]);
> + data->num_handles, handle);
>
> - attr = gatt_db_get_attribute(server->db, data->handles[0]);
> + attr = gatt_db_get_attribute(server->db, handle);
>
> if (!attr) {
> ecode = BT_ATT_ERROR_INVALID_HANDLE;
> goto error;
> }
>
> + ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
> + BT_ATT_PERM_READ_AUTHEN |
> + BT_ATT_PERM_READ_ENCRYPT);
> + if (ecode)
> + goto error;
> +
> if (gatt_db_attribute_read(attr, 0, opcode, server->att,
> read_multiple_complete_cb, data))
> return;
>
> + ecode = BT_ATT_ERROR_UNLIKELY;
> +
> error:
> if (data)
> read_mult_data_free(data);
>
> - bt_att_chan_send_error_rsp(chan, opcode, 0, ecode);
> + bt_att_chan_send_error_rsp(chan, opcode, handle, ecode);
> }
>
> static bool append_prep_data(struct prep_write_data *prep_data, uint16_t handle,
> --
> 2.26.2.526.g744177e7f7-goog
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Bluez PATCH v1] shared/gatt-server: Fix read multiple charc values
2020-05-06 17:28 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
@ 2020-05-07 2:02 ` Archie Pusaka
0 siblings, 0 replies; 4+ messages in thread
From: Archie Pusaka @ 2020-05-07 2:02 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
Hi Luiz,
On Thu, 7 May 2020 at 01:28, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Archie,
>
> On Wed, May 6, 2020 at 4:45 AM Archie Pusaka <apusaka@google.com> wrote:
> >
> > From: Archie Pusaka <apusaka@chromium.org>
> >
> > According to bluetooth spec Ver 5.2, Vol 3, Part G, 4.8.4, An
> > ATT_ERROR_RSP PDU shall be sent by the server in response to the
> > ATT_READ_MULTIPLE_RSP PDU if insufficient authentication,
> > insufficient authorization, insufficient encryption key size, or
> > insufficient encryption is used by the client, or if a read operation
> > is not permitted on any of the Characteristic Values.
> >
> > Currently if the size of the response grows larger than the MTU size,
> > BlueZ does an early return and not check the permission for the rest
> > of the characteristics. This patch forces BlueZ to check for possible
> > errors even though we already reach MTU size.
> >
> > This patch also moves the read permission check for read multiple
> > characteristics so it is done before actually retrieving the
> > characteristics.
>
> Is there a test failing because of this? If there is not I would have
> thought this is actually a prefered behavior since the subsequent read
> would cause an error at least the client would be able to the data it
> could read in the beginning otherwise this will be imposing the
> database to read all the handles discarding the data that don't fit
> just to propagate the errors before the data can actually be read.
>
I was running the qualification test GATT/SR/GAR/BI-18-C to BI-22-C,
and sometimes this issue pops out.
It looks like PTS runs these tests by randomly selecting 1 readable
characteristic and 1 other characteristic which is known to be
unauthorized, nonexistent, or with other types of errors. If the 1
readable characteristic is too long, then BlueZ will not return the
expected error, which upsets PTS.
The core spec also describes this behavior in Vol 3, Part F, 3.4.4.7:
"The server shall respond with an ATT_READ_MULTIPLE_RSP PDU if all the
handles are valid and all attributes have sufficient permissions to
allow reading."
> > ---
> >
> > src/shared/gatt-server.c | 88 ++++++++++++++++++++--------------------
> > 1 file changed, 45 insertions(+), 43 deletions(-)
> >
> > diff --git a/src/shared/gatt-server.c b/src/shared/gatt-server.c
> > index 4e07398d2..e937d80a8 100644
> > --- a/src/shared/gatt-server.c
> > +++ b/src/shared/gatt-server.c
> > @@ -1057,33 +1057,23 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
> > uint16_t length;
> >
> > if (err != 0) {
> > - bt_att_chan_send_error_rsp(data->chan, data->opcode, handle,
> > - err);
> > - read_mult_data_free(data);
> > - return;
> > - }
> > -
> > - ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
> > - BT_ATT_PERM_READ_AUTHEN |
> > - BT_ATT_PERM_READ_ENCRYPT);
> > - if (ecode) {
> > - bt_att_chan_send_error_rsp(data->chan, data->opcode, handle,
> > - ecode);
> > - read_mult_data_free(data);
> > - return;
> > + ecode = err;
> > + goto error;
> > }
> >
> > length = data->opcode == BT_ATT_OP_READ_MULT_VL_REQ ?
> > - MIN(len, data->mtu - data->length - 3) :
> > + MIN(len, MAX(0, data->mtu - data->length - 3)) :
> > MIN(len, data->mtu - data->length - 1);
> >
> > if (data->opcode == BT_ATT_OP_READ_MULT_VL_REQ) {
> > /* The Length Value Tuple List may be truncated within the first
> > * two octets of a tuple due to the size limits of the current
> > - * ATT_MTU.
> > + * ATT_MTU, but the first two octets cannot be separated.
> > */
> > - put_le16(len, data->rsp_data + data->length);
> > - data->length += 2;
> > + if (data->mtu - data->length >= 3) {
> > + put_le16(len, data->rsp_data + data->length);
> > + data->length += 2;
> > + }
> > }
> >
> > memcpy(data->rsp_data + data->length, value, length);
> > @@ -1091,45 +1081,46 @@ static void read_multiple_complete_cb(struct gatt_db_attribute *attr, int err,
> >
> > data->cur_handle++;
> >
> > - len = data->opcode == BT_ATT_OP_READ_MULT_VL_REQ ?
> > - data->mtu - data->length - 3 : data->mtu - data->length - 1;
> > -
> > - if (!len || (data->cur_handle == data->num_handles)) {
> > + if (data->cur_handle == data->num_handles) {
> > bt_att_chan_send_rsp(data->chan, data->opcode + 1,
> > data->rsp_data, data->length);
> > read_mult_data_free(data);
> > return;
> > }
> >
> > + handle = data->handles[data->cur_handle];
> > +
> > util_debug(data->server->debug_callback, data->server->debug_data,
> > "%s Req - #%zu of %zu: 0x%04x",
> > data->opcode == BT_ATT_OP_READ_MULT_REQ ?
> > "Read Multiple" :
> > "Read Multiple Variable Length",
> > data->cur_handle + 1, data->num_handles,
> > - data->handles[data->cur_handle]);
> > + handle);
> >
> > - next_attr = gatt_db_get_attribute(data->server->db,
> > - data->handles[data->cur_handle]);
> > + next_attr = gatt_db_get_attribute(data->server->db, handle);
> >
> > if (!next_attr) {
> > - bt_att_chan_send_error_rsp(data->chan,
> > - BT_ATT_OP_READ_MULT_REQ,
> > - data->handles[data->cur_handle],
> > - BT_ATT_ERROR_INVALID_HANDLE);
> > - read_mult_data_free(data);
> > - return;
> > + ecode = BT_ATT_ERROR_INVALID_HANDLE;
> > + goto error;
> > }
> >
> > - if (!gatt_db_attribute_read(next_attr, 0, BT_ATT_OP_READ_MULT_REQ,
> > + ecode = check_permissions(data->server, next_attr, BT_ATT_PERM_READ |
> > + BT_ATT_PERM_READ_AUTHEN |
> > + BT_ATT_PERM_READ_ENCRYPT);
> > + if (ecode)
> > + goto error;
> > +
> > + if (gatt_db_attribute_read(next_attr, 0, data->opcode,
> > data->server->att,
> > - read_multiple_complete_cb, data)) {
> > - bt_att_chan_send_error_rsp(data->chan,
> > - BT_ATT_OP_READ_MULT_REQ,
> > - data->handles[data->cur_handle],
> > - BT_ATT_ERROR_UNLIKELY);
> > - read_mult_data_free(data);
> > - }
> > + read_multiple_complete_cb, data))
> > + return;
> > +
> > + ecode = BT_ATT_ERROR_UNLIKELY;
> > +
> > +error:
> > + bt_att_chan_send_error_rsp(data->chan, data->opcode, handle, ecode);
> > + read_mult_data_free(data);
> > }
> >
> > static struct read_mult_data *read_mult_data_new(struct bt_gatt_server *server,
> > @@ -1161,8 +1152,9 @@ static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
> > struct bt_gatt_server *server = user_data;
> > struct gatt_db_attribute *attr;
> > struct read_mult_data *data = NULL;
> > - uint8_t ecode = BT_ATT_ERROR_UNLIKELY;
> > + uint8_t ecode;
> > size_t i = 0;
> > + uint16_t handle = 0;
> >
> > if (length < 4) {
> > ecode = BT_ATT_ERROR_INVALID_PDU;
> > @@ -1176,28 +1168,38 @@ static void read_multiple_cb(struct bt_att_chan *chan, uint8_t opcode,
> > for (i = 0; i < data->num_handles; i++)
> > data->handles[i] = get_le16(pdu + i * 2);
> >
> > + handle = data->handles[0];
> > +
> > util_debug(server->debug_callback, server->debug_data,
> > "%s Req - %zu handles, 1st: 0x%04x",
> > data->opcode == BT_ATT_OP_READ_MULT_REQ ?
> > "Read Multiple" : "Read Multiple Variable Length",
> > - data->num_handles, data->handles[0]);
> > + data->num_handles, handle);
> >
> > - attr = gatt_db_get_attribute(server->db, data->handles[0]);
> > + attr = gatt_db_get_attribute(server->db, handle);
> >
> > if (!attr) {
> > ecode = BT_ATT_ERROR_INVALID_HANDLE;
> > goto error;
> > }
> >
> > + ecode = check_permissions(data->server, attr, BT_ATT_PERM_READ |
> > + BT_ATT_PERM_READ_AUTHEN |
> > + BT_ATT_PERM_READ_ENCRYPT);
> > + if (ecode)
> > + goto error;
> > +
> > if (gatt_db_attribute_read(attr, 0, opcode, server->att,
> > read_multiple_complete_cb, data))
> > return;
> >
> > + ecode = BT_ATT_ERROR_UNLIKELY;
> > +
> > error:
> > if (data)
> > read_mult_data_free(data);
> >
> > - bt_att_chan_send_error_rsp(chan, opcode, 0, ecode);
> > + bt_att_chan_send_error_rsp(chan, opcode, handle, ecode);
> > }
> >
> > static bool append_prep_data(struct prep_write_data *prep_data, uint16_t handle,
> > --
> > 2.26.2.526.g744177e7f7-goog
> >
>
>
> --
> Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-05-07 2:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 11:44 [Bluez PATCH v1] shared/gatt-server: Fix read multiple charc values Archie Pusaka
2020-05-06 12:09 ` [Bluez,v1] " bluez.test.bot
2020-05-06 17:28 ` [Bluez PATCH v1] " Luiz Augusto von Dentz
2020-05-07 2:02 ` Archie Pusaka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).