* [PATCH BlueZ] client: Add possiblity to define maximum attribute value length
@ 2018-04-27 14:01 Grzegorz Kolodziejczyk
2018-04-27 14:01 ` [PATCH BlueZ] client: Define maximum attribute value length as initial value Grzegorz Kolodziejczyk
2018-04-27 14:19 ` [PATCH BlueZ] client: Add possiblity to define maximum attribute value length Grzegorz Kołodziejczyk
0 siblings, 2 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-04-27 14:01 UTC (permalink / raw)
To: linux-bluetooth
This patch allows to define maximum value length for characteristic and
descriptor value while registering.
---
client/gatt.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++------------
client/gatt.h | 4 ++++
client/main.c | 13 ++++++------
3 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index d59d1ba1e..969a78be0 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -60,6 +60,7 @@ struct desc {
char *uuid;
char **flags;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
};
@@ -71,6 +72,7 @@ struct chrc {
bool notifying;
GList *descs;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
uint16_t mtu;
struct io *write_io;
@@ -612,7 +614,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
{
struct iovec iov;
struct write_attribute_data wd;
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -689,7 +691,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
static bool pipe_read(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
- uint8_t buf[512];
+ uint8_t buf[MAX_ATTR_VAL_LEN];
int fd = io_get_fd(io);
ssize_t bytes_read;
@@ -1611,7 +1613,8 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
}
-static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
+static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
+ int max_len)
{
DBusMessageIter array;
uint16_t offset = 0;
@@ -1628,6 +1631,9 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
if (parse_options(iter, &offset, NULL, NULL, NULL))
return -EINVAL;
+ if ((offset + read_len) > max_len)
+ return -EOVERFLOW;
+
if ((offset + read_len) > *len) {
*len = offset + read_len;
*value = g_realloc(*value, *len);
@@ -1645,6 +1651,7 @@ static void authorize_write_response(const char *input, void *user_data)
DBusMessageIter iter;
DBusMessage *reply;
char *err;
+ int errsv;
dbus_message_iter_init(pending_message, &iter);
@@ -1656,10 +1663,16 @@ static void authorize_write_response(const char *input, void *user_data)
chrc->authorized = true;
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
err = "org.bluez.Error.InvalidArguments";
goto error;
+ } else if (errsv == -EOVERFLOW) {
+ err = "org.bluez.Error.InvalidValueLength";
+
+ goto error;
}
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1685,6 +1698,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
struct chrc *chrc = user_data;
DBusMessageIter iter;
char *str;
+ int errsv;
dbus_message_iter_init(msg, &iter);
@@ -1706,10 +1720,15 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidArguments", NULL);
+ } else if (errsv == -EOVERFLOW) {
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidValueLength", NULL);
+ }
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1881,9 +1900,9 @@ static const GDBusMethodTable chrc_methods[] = {
{ }
};
-static uint8_t *str2bytearray(char *arg, int *val_len)
+static uint8_t *str2bytearray(char *arg, int *val_len, unsigned int max_val_len)
{
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -1908,6 +1927,12 @@ static uint8_t *str2bytearray(char *arg, int *val_len)
value[i] = val;
}
+ if (i > max_val_len) {
+ bt_shell_printf("Value exceeds maximum length (%i)\n",
+ max_val_len);
+ return NULL;
+ }
+
*val_len = i;
return g_memdup(value, i);
@@ -1919,7 +1944,13 @@ static void chrc_set_value(const char *input, void *user_data)
g_free(chrc->value);
- chrc->value = str2bytearray((char *) input, &chrc->value_len);
+ chrc->value = str2bytearray((char *) input, &chrc->value_len,
+ chrc->max_val_len);
+
+ if (!chrc->value) {
+ print_chrc(chrc, COLORED_DEL);
+ chrc_unregister(chrc);
+ }
}
void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
@@ -1940,7 +1971,8 @@ void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
chrc->uuid = g_strdup(argv[1]);
chrc->path = g_strdup_printf("%s/chrc%p", service->path, chrc);
chrc->flags = g_strsplit(argv[2], ",", -1);
- chrc->authorization_req = argc > 3 ? true : false;
+ chrc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
+ chrc->authorization_req = argc > 4 ? true : false;
if (g_dbus_register_interface(conn, chrc->path, CHRC_INTERFACE,
chrc_methods, NULL, chrc_properties,
@@ -2037,7 +2069,8 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_value_arg(&iter, &desc->value, &desc->value_len))
+ if (parse_value_arg(&iter, &desc->value, &desc->value_len,
+ desc->max_val_len))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2140,7 +2173,13 @@ static void desc_set_value(const char *input, void *user_data)
g_free(desc->value);
- desc->value = str2bytearray((char *) input, &desc->value_len);
+ desc->value = str2bytearray((char *) input, &desc->value_len,
+ desc->max_val_len);
+
+ if (!desc->value) {
+ print_desc(desc, COLORED_DEL);
+ desc_unregister(desc);
+ }
}
void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
@@ -2166,6 +2205,7 @@ void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
desc->uuid = g_strdup(argv[1]);
desc->path = g_strdup_printf("%s/desc%p", desc->chrc->path, desc);
desc->flags = g_strsplit(argv[2], ",", -1);
+ desc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
if (g_dbus_register_interface(conn, desc->path, DESC_INTERFACE,
desc_methods, NULL, desc_properties,
diff --git a/client/gatt.h b/client/gatt.h
index 957ae8003..189a0aa40 100644
--- a/client/gatt.h
+++ b/client/gatt.h
@@ -21,6 +21,10 @@
*
*/
+#define MAX_ATTR_VAL_LEN 512
+#define STR_(i) #i
+#define STR(i) STR_(i)
+
void gatt_add_service(GDBusProxy *proxy);
void gatt_remove_service(GDBusProxy *proxy);
diff --git a/client/main.c b/client/main.c
index dd85a1c85..60e0f10b0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2013,7 +2013,7 @@ static void cmd_register_characteristic(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return bt_shell_noninteractive_quit(EXIT_FAILURE);
- if (argc > 3 && strcmp(argv[3], "authorize")) {
+ if (argc > 4 && strcmp(argv[4], "authorize")) {
bt_shell_printf("Invalid authorize argument\n");
return bt_shell_noninteractive_quit(EXIT_FAILURE);
}
@@ -2437,14 +2437,15 @@ static const struct bt_shell_menu gatt_menu = {
{ "unregister-service", "<UUID/object>", cmd_unregister_service,
"Unregister application service" },
{ "register-characteristic", "<UUID> <Flags=read,write,notify...> "
- "[authorize]", cmd_register_characteristic,
- "Register application characteristic" },
+ "[max value len=0(default=" STR(MAX_ATTR_VAL_LEN) ")-"
+ STR(MAX_ATTR_VAL_LEN) "] [authorize]",
+ cmd_register_characteristic,
+ "Register application characteristic" },
{ "unregister-characteristic", "<UUID/object>",
cmd_unregister_characteristic,
"Unregister application characteristic" },
- { "register-descriptor", "<UUID> <Flags=read,write...>",
- cmd_register_descriptor,
- "Register application descriptor" },
+ { "register-descriptor", "<UUID> <Flags=read,write...> [max value len]",
+ cmd_register_descriptor, "Register application descriptor" },
{ "unregister-descriptor", "<UUID/object>",
cmd_unregister_descriptor,
"Unregister application descriptor" },
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH BlueZ] client: Define maximum attribute value length as initial value
2018-04-27 14:01 [PATCH BlueZ] client: Add possiblity to define maximum attribute value length Grzegorz Kolodziejczyk
@ 2018-04-27 14:01 ` Grzegorz Kolodziejczyk
2018-04-27 14:19 ` [PATCH BlueZ] client: Add possiblity to define maximum attribute value length Grzegorz Kołodziejczyk
1 sibling, 0 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-04-27 14:01 UTC (permalink / raw)
To: linux-bluetooth
Initial registered attribute value is set as maximal attribute length.
---
client/gatt.c | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 45 insertions(+), 9 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index d59d1ba1e..6bf644c48 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -54,12 +54,15 @@
#define COLORED_CHG COLOR_YELLOW "CHG" COLOR_OFF
#define COLORED_DEL COLOR_RED "DEL" COLOR_OFF
+#define MAX_ATTR_VAL_LEN 512
+
struct desc {
struct chrc *chrc;
char *path;
char *uuid;
char **flags;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
};
@@ -71,6 +74,7 @@ struct chrc {
bool notifying;
GList *descs;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
uint16_t mtu;
struct io *write_io;
@@ -612,7 +616,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
{
struct iovec iov;
struct write_attribute_data wd;
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -689,7 +693,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
static bool pipe_read(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
- uint8_t buf[512];
+ uint8_t buf[MAX_ATTR_VAL_LEN];
int fd = io_get_fd(io);
ssize_t bytes_read;
@@ -1611,7 +1615,8 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
}
-static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
+static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
+ int max_len)
{
DBusMessageIter array;
uint16_t offset = 0;
@@ -1628,6 +1633,9 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
if (parse_options(iter, &offset, NULL, NULL, NULL))
return -EINVAL;
+ if ((offset + read_len) > max_len)
+ return -EOVERFLOW;
+
if ((offset + read_len) > *len) {
*len = offset + read_len;
*value = g_realloc(*value, *len);
@@ -1645,6 +1653,7 @@ static void authorize_write_response(const char *input, void *user_data)
DBusMessageIter iter;
DBusMessage *reply;
char *err;
+ int errsv;
dbus_message_iter_init(pending_message, &iter);
@@ -1656,10 +1665,16 @@ static void authorize_write_response(const char *input, void *user_data)
chrc->authorized = true;
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
err = "org.bluez.Error.InvalidArguments";
goto error;
+ } else if (errsv == -EOVERFLOW) {
+ err = "org.bluez.Error.InvalidValueLength";
+
+ goto error;
}
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1685,6 +1700,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
struct chrc *chrc = user_data;
DBusMessageIter iter;
char *str;
+ int errsv;
dbus_message_iter_init(msg, &iter);
@@ -1706,10 +1722,15 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidArguments", NULL);
+ } else if (errsv == -EOVERFLOW) {
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidValueLength", NULL);
+ }
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1883,7 +1904,7 @@ static const GDBusMethodTable chrc_methods[] = {
static uint8_t *str2bytearray(char *arg, int *val_len)
{
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -1920,6 +1941,13 @@ static void chrc_set_value(const char *input, void *user_data)
g_free(chrc->value);
chrc->value = str2bytearray((char *) input, &chrc->value_len);
+
+ if (!chrc->value) {
+ print_chrc(chrc, COLORED_DEL);
+ chrc_unregister(chrc);
+ }
+
+ chrc->max_val_len = chrc->value_len;
}
void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
@@ -2037,7 +2065,8 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_value_arg(&iter, &desc->value, &desc->value_len))
+ if (parse_value_arg(&iter, &desc->value, &desc->value_len,
+ desc->max_val_len))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2141,6 +2170,13 @@ static void desc_set_value(const char *input, void *user_data)
g_free(desc->value);
desc->value = str2bytearray((char *) input, &desc->value_len);
+
+ if (!desc->value) {
+ print_desc(desc, COLORED_DEL);
+ desc_unregister(desc);
+ }
+
+ desc->max_val_len = desc->value_len;
}
void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH BlueZ] client: Add possiblity to define maximum attribute value length
2018-04-27 14:01 [PATCH BlueZ] client: Add possiblity to define maximum attribute value length Grzegorz Kolodziejczyk
2018-04-27 14:01 ` [PATCH BlueZ] client: Define maximum attribute value length as initial value Grzegorz Kolodziejczyk
@ 2018-04-27 14:19 ` Grzegorz Kołodziejczyk
1 sibling, 0 replies; 6+ messages in thread
From: Grzegorz Kołodziejczyk @ 2018-04-27 14:19 UTC (permalink / raw)
To: linux-bluetooth
Hi,
pt., 27 kwi 2018 o 16:01 Grzegorz Kolodziejczyk <
grzegorz.kolodziejczyk@codecoup.pl> napisa=C5=82(a):
> This patch allows to define maximum value length for characteristic and
> descriptor value while registering.
> ---
> client/gatt.c | 66
+++++++++++++++++++++++++++++++++++++++++++++++------------
> client/gatt.h | 4 ++++
> client/main.c | 13 ++++++------
> 3 files changed, 64 insertions(+), 19 deletions(-)
> diff --git a/client/gatt.c b/client/gatt.c
> index d59d1ba1e..969a78be0 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -60,6 +60,7 @@ struct desc {
> char *uuid;
> char **flags;
> int value_len;
> + unsigned int max_val_len;
> uint8_t *value;
> };
> @@ -71,6 +72,7 @@ struct chrc {
> bool notifying;
> GList *descs;
> int value_len;
> + unsigned int max_val_len;
> uint8_t *value;
> uint16_t mtu;
> struct io *write_io;
> @@ -612,7 +614,7 @@ static void write_attribute(GDBusProxy *proxy, char
*val_str, uint16_t offset)
> {
> struct iovec iov;
> struct write_attribute_data wd;
> - uint8_t value[512];
> + uint8_t value[MAX_ATTR_VAL_LEN];
> char *entry;
> unsigned int i;
> @@ -689,7 +691,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int
argc, char *argv[])
> static bool pipe_read(struct io *io, void *user_data)
> {
> struct chrc *chrc =3D user_data;
> - uint8_t buf[512];
> + uint8_t buf[MAX_ATTR_VAL_LEN];
> int fd =3D io_get_fd(io);
> ssize_t bytes_read;
> @@ -1611,7 +1613,8 @@ static DBusMessage *chrc_read_value(DBusConnection
*conn, DBusMessage *msg,
> return read_value(msg, &chrc->value[offset], chrc->value_len -
offset);
> }
> -static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int
*len)
> +static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int
*len,
> + int
max_len)
> {
> DBusMessageIter array;
> uint16_t offset =3D 0;
> @@ -1628,6 +1631,9 @@ static int parse_value_arg(DBusMessageIter *iter,
uint8_t **value, int *len)
> if (parse_options(iter, &offset, NULL, NULL, NULL))
> return -EINVAL;
> + if ((offset + read_len) > max_len)
> + return -EOVERFLOW;
> +
> if ((offset + read_len) > *len) {
> *len =3D offset + read_len;
> *value =3D g_realloc(*value, *len);
> @@ -1645,6 +1651,7 @@ static void authorize_write_response(const char
*input, void *user_data)
> DBusMessageIter iter;
> DBusMessage *reply;
> char *err;
> + int errsv;
> dbus_message_iter_init(pending_message, &iter);
> @@ -1656,10 +1663,16 @@ static void authorize_write_response(const char
*input, void *user_data)
> chrc->authorized =3D true;
> - if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
> + errsv =3D parse_value_arg(&iter, &chrc->value, &chrc->value_len,
> +
chrc->max_val_len);
> + if (errsv =3D=3D -EINVAL) {
> err =3D "org.bluez.Error.InvalidArguments";
> goto error;
> + } else if (errsv =3D=3D -EOVERFLOW) {
> + err =3D "org.bluez.Error.InvalidValueLength";
> +
> + goto error;
> }
> bt_shell_printf("[" COLORED_CHG "] Attribute %s written" ,
chrc->path);
> @@ -1685,6 +1698,7 @@ static DBusMessage *chrc_write_value(DBusConnection
*conn, DBusMessage *msg,
> struct chrc *chrc =3D user_data;
> DBusMessageIter iter;
> char *str;
> + int errsv;
> dbus_message_iter_init(msg, &iter);
> @@ -1706,10 +1720,15 @@ static DBusMessage
*chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> return NULL;
> }
> - if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
> + errsv =3D parse_value_arg(&iter, &chrc->value, &chrc->value_len,
> +
chrc->max_val_len);
> + if (errsv =3D=3D -EINVAL) {
> return g_dbus_create_error(msg,
> -
"org.bluez.Error.InvalidArguments",
> - NULL);
> + "org.bluez.Error.InvalidArguments", NULL)=
;
> + } else if (errsv =3D=3D -EOVERFLOW) {
> + return g_dbus_create_error(msg,
> + "org.bluez.Error.InvalidValueLength",
NULL);
> + }
> bt_shell_printf("[" COLORED_CHG "] Attribute %s written" ,
chrc->path);
> @@ -1881,9 +1900,9 @@ static const GDBusMethodTable chrc_methods[] =3D {
> { }
> };
> -static uint8_t *str2bytearray(char *arg, int *val_len)
> +static uint8_t *str2bytearray(char *arg, int *val_len, unsigned int
max_val_len)
> {
> - uint8_t value[512];
> + uint8_t value[MAX_ATTR_VAL_LEN];
> char *entry;
> unsigned int i;
> @@ -1908,6 +1927,12 @@ static uint8_t *str2bytearray(char *arg, int
*val_len)
> value[i] =3D val;
> }
> + if (i > max_val_len) {
> + bt_shell_printf("Value exceeds maximum length (%i)\n",
> +
max_val_len);
> + return NULL;
> + }
> +
> *val_len =3D i;
> return g_memdup(value, i);
> @@ -1919,7 +1944,13 @@ static void chrc_set_value(const char *input, void
*user_data)
> g_free(chrc->value);
> - chrc->value =3D str2bytearray((char *) input, &chrc->value_len);
> + chrc->value =3D str2bytearray((char *) input, &chrc->value_len,
> +
chrc->max_val_len);
> +
> + if (!chrc->value) {
> + print_chrc(chrc, COLORED_DEL);
> + chrc_unregister(chrc);
> + }
> }
> void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
> @@ -1940,7 +1971,8 @@ void gatt_register_chrc(DBusConnection *conn,
GDBusProxy *proxy,
> chrc->uuid =3D g_strdup(argv[1]);
> chrc->path =3D g_strdup_printf("%s/chrc%p", service->path, chrc=
);
> chrc->flags =3D g_strsplit(argv[2], ",", -1);
> - chrc->authorization_req =3D argc > 3 ? true : false;
> + chrc->max_val_len =3D argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN=
;
> + chrc->authorization_req =3D argc > 4 ? true : false;
> if (g_dbus_register_interface(conn, chrc->path, CHRC_INTERFACE,
> chrc_methods, NULL,
chrc_properties,
> @@ -2037,7 +2069,8 @@ static DBusMessage *desc_write_value(DBusConnection
*conn, DBusMessage *msg,
> dbus_message_iter_init(msg, &iter);
> - if (parse_value_arg(&iter, &desc->value, &desc->value_len))
> + if (parse_value_arg(&iter, &desc->value, &desc->value_len,
> +
desc->max_val_len))
> return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
> NULL);
> @@ -2140,7 +2173,13 @@ static void desc_set_value(const char *input, void
*user_data)
> g_free(desc->value);
> - desc->value =3D str2bytearray((char *) input, &desc->value_len);
> + desc->value =3D str2bytearray((char *) input, &desc->value_len,
> +
desc->max_val_len);
> +
> + if (!desc->value) {
> + print_desc(desc, COLORED_DEL);
> + desc_unregister(desc);
> + }
> }
> void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
> @@ -2166,6 +2205,7 @@ void gatt_register_desc(DBusConnection *conn,
GDBusProxy *proxy,
> desc->uuid =3D g_strdup(argv[1]);
> desc->path =3D g_strdup_printf("%s/desc%p", desc->chrc->path,
desc);
> desc->flags =3D g_strsplit(argv[2], ",", -1);
> + desc->max_val_len =3D argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN=
;
> if (g_dbus_register_interface(conn, desc->path, DESC_INTERFACE,
> desc_methods, NULL,
desc_properties,
> diff --git a/client/gatt.h b/client/gatt.h
> index 957ae8003..189a0aa40 100644
> --- a/client/gatt.h
> +++ b/client/gatt.h
> @@ -21,6 +21,10 @@
> *
> */
> +#define MAX_ATTR_VAL_LEN 512
> +#define STR_(i) #i
> +#define STR(i) STR_(i)
> +
> void gatt_add_service(GDBusProxy *proxy);
> void gatt_remove_service(GDBusProxy *proxy);
> diff --git a/client/main.c b/client/main.c
> index dd85a1c85..60e0f10b0 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -2013,7 +2013,7 @@ static void cmd_register_characteristic(int argc,
char *argv[])
> if (check_default_ctrl() =3D=3D FALSE)
> return bt_shell_noninteractive_quit(EXIT_FAILURE);
> - if (argc > 3 && strcmp(argv[3], "authorize")) {
> + if (argc > 4 && strcmp(argv[4], "authorize")) {
> bt_shell_printf("Invalid authorize argument\n");
> return bt_shell_noninteractive_quit(EXIT_FAILURE);
> }
> @@ -2437,14 +2437,15 @@ static const struct bt_shell_menu gatt_menu =3D {
> { "unregister-service", "<UUID/object>", cmd_unregister_service=
,
> "Unregister application service=
"
},
> { "register-characteristic", "<UUID> <Flags=3Dread,write,notify=
...>
"
> - "[authorize]",
cmd_register_characteristic,
> - "Register application characteristic" },
> + "[max value len=3D0(default=3D" STR(MAX_ATTR_VAL_=
LEN)
")-"
> + STR(MAX_ATTR_VAL_LEN) "] [authorize]",
> + cmd_register_characteristic,
> + "Register application characteristic" },
> { "unregister-characteristic", "<UUID/object>",
> cmd_unregister_characteristic,
> "Unregister application characteristic"
},
> - { "register-descriptor", "<UUID> <Flags=3Dread,write...>",
> - cmd_register_descriptor,
> - "Register application descriptor"
},
> + { "register-descriptor", "<UUID> <Flags=3Dread,write...> [max val=
ue
len]",
> + cmd_register_descriptor, "Register application
descriptor" },
> { "unregister-descriptor", "<UUID/object>",
> cmd_unregister_descriptor,
> "Unregister application
descriptor" },
> --
> 2.13.6
Please ommit this - it's send by accident.
Regards,
Grzegorz
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH BlueZ] client: Add possiblity to define maximum attribute value length
@ 2018-04-27 14:01 Grzegorz Kolodziejczyk
0 siblings, 0 replies; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-04-27 14:01 UTC (permalink / raw)
To: linux-bluetooth
This patch allows to define maximum value length for characteristic and
descriptor value while registering.
---
client/gatt.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++------------
client/gatt.h | 4 ++++
client/main.c | 13 ++++++------
3 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index d59d1ba1e..969a78be0 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -60,6 +60,7 @@ struct desc {
char *uuid;
char **flags;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
};
@@ -71,6 +72,7 @@ struct chrc {
bool notifying;
GList *descs;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
uint16_t mtu;
struct io *write_io;
@@ -612,7 +614,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
{
struct iovec iov;
struct write_attribute_data wd;
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -689,7 +691,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
static bool pipe_read(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
- uint8_t buf[512];
+ uint8_t buf[MAX_ATTR_VAL_LEN];
int fd = io_get_fd(io);
ssize_t bytes_read;
@@ -1611,7 +1613,8 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
}
-static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
+static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
+ int max_len)
{
DBusMessageIter array;
uint16_t offset = 0;
@@ -1628,6 +1631,9 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
if (parse_options(iter, &offset, NULL, NULL, NULL))
return -EINVAL;
+ if ((offset + read_len) > max_len)
+ return -EOVERFLOW;
+
if ((offset + read_len) > *len) {
*len = offset + read_len;
*value = g_realloc(*value, *len);
@@ -1645,6 +1651,7 @@ static void authorize_write_response(const char *input, void *user_data)
DBusMessageIter iter;
DBusMessage *reply;
char *err;
+ int errsv;
dbus_message_iter_init(pending_message, &iter);
@@ -1656,10 +1663,16 @@ static void authorize_write_response(const char *input, void *user_data)
chrc->authorized = true;
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
err = "org.bluez.Error.InvalidArguments";
goto error;
+ } else if (errsv == -EOVERFLOW) {
+ err = "org.bluez.Error.InvalidValueLength";
+
+ goto error;
}
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1685,6 +1698,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
struct chrc *chrc = user_data;
DBusMessageIter iter;
char *str;
+ int errsv;
dbus_message_iter_init(msg, &iter);
@@ -1706,10 +1720,15 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidArguments", NULL);
+ } else if (errsv == -EOVERFLOW) {
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidValueLength", NULL);
+ }
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1881,9 +1900,9 @@ static const GDBusMethodTable chrc_methods[] = {
{ }
};
-static uint8_t *str2bytearray(char *arg, int *val_len)
+static uint8_t *str2bytearray(char *arg, int *val_len, unsigned int max_val_len)
{
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -1908,6 +1927,12 @@ static uint8_t *str2bytearray(char *arg, int *val_len)
value[i] = val;
}
+ if (i > max_val_len) {
+ bt_shell_printf("Value exceeds maximum length (%i)\n",
+ max_val_len);
+ return NULL;
+ }
+
*val_len = i;
return g_memdup(value, i);
@@ -1919,7 +1944,13 @@ static void chrc_set_value(const char *input, void *user_data)
g_free(chrc->value);
- chrc->value = str2bytearray((char *) input, &chrc->value_len);
+ chrc->value = str2bytearray((char *) input, &chrc->value_len,
+ chrc->max_val_len);
+
+ if (!chrc->value) {
+ print_chrc(chrc, COLORED_DEL);
+ chrc_unregister(chrc);
+ }
}
void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
@@ -1940,7 +1971,8 @@ void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
chrc->uuid = g_strdup(argv[1]);
chrc->path = g_strdup_printf("%s/chrc%p", service->path, chrc);
chrc->flags = g_strsplit(argv[2], ",", -1);
- chrc->authorization_req = argc > 3 ? true : false;
+ chrc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
+ chrc->authorization_req = argc > 4 ? true : false;
if (g_dbus_register_interface(conn, chrc->path, CHRC_INTERFACE,
chrc_methods, NULL, chrc_properties,
@@ -2037,7 +2069,8 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_value_arg(&iter, &desc->value, &desc->value_len))
+ if (parse_value_arg(&iter, &desc->value, &desc->value_len,
+ desc->max_val_len))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2140,7 +2173,13 @@ static void desc_set_value(const char *input, void *user_data)
g_free(desc->value);
- desc->value = str2bytearray((char *) input, &desc->value_len);
+ desc->value = str2bytearray((char *) input, &desc->value_len,
+ desc->max_val_len);
+
+ if (!desc->value) {
+ print_desc(desc, COLORED_DEL);
+ desc_unregister(desc);
+ }
}
void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
@@ -2166,6 +2205,7 @@ void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
desc->uuid = g_strdup(argv[1]);
desc->path = g_strdup_printf("%s/desc%p", desc->chrc->path, desc);
desc->flags = g_strsplit(argv[2], ",", -1);
+ desc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
if (g_dbus_register_interface(conn, desc->path, DESC_INTERFACE,
desc_methods, NULL, desc_properties,
diff --git a/client/gatt.h b/client/gatt.h
index 957ae8003..189a0aa40 100644
--- a/client/gatt.h
+++ b/client/gatt.h
@@ -21,6 +21,10 @@
*
*/
+#define MAX_ATTR_VAL_LEN 512
+#define STR_(i) #i
+#define STR(i) STR_(i)
+
void gatt_add_service(GDBusProxy *proxy);
void gatt_remove_service(GDBusProxy *proxy);
diff --git a/client/main.c b/client/main.c
index dd85a1c85..60e0f10b0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2013,7 +2013,7 @@ static void cmd_register_characteristic(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return bt_shell_noninteractive_quit(EXIT_FAILURE);
- if (argc > 3 && strcmp(argv[3], "authorize")) {
+ if (argc > 4 && strcmp(argv[4], "authorize")) {
bt_shell_printf("Invalid authorize argument\n");
return bt_shell_noninteractive_quit(EXIT_FAILURE);
}
@@ -2437,14 +2437,15 @@ static const struct bt_shell_menu gatt_menu = {
{ "unregister-service", "<UUID/object>", cmd_unregister_service,
"Unregister application service" },
{ "register-characteristic", "<UUID> <Flags=read,write,notify...> "
- "[authorize]", cmd_register_characteristic,
- "Register application characteristic" },
+ "[max value len=0(default=" STR(MAX_ATTR_VAL_LEN) ")-"
+ STR(MAX_ATTR_VAL_LEN) "] [authorize]",
+ cmd_register_characteristic,
+ "Register application characteristic" },
{ "unregister-characteristic", "<UUID/object>",
cmd_unregister_characteristic,
"Unregister application characteristic" },
- { "register-descriptor", "<UUID> <Flags=read,write...>",
- cmd_register_descriptor,
- "Register application descriptor" },
+ { "register-descriptor", "<UUID> <Flags=read,write...> [max value len]",
+ cmd_register_descriptor, "Register application descriptor" },
{ "unregister-descriptor", "<UUID/object>",
cmd_unregister_descriptor,
"Unregister application descriptor" },
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH BlueZ] client: Add possiblity to define maximum attribute value length
@ 2018-04-26 14:55 Grzegorz Kolodziejczyk
2018-04-26 16:05 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 6+ messages in thread
From: Grzegorz Kolodziejczyk @ 2018-04-26 14:55 UTC (permalink / raw)
To: linux-bluetooth
This patch allows to define maximum value length for characteristic and
descriptor value while registering.
---
client/gatt.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++------------
client/gatt.h | 4 ++++
client/main.c | 13 ++++++------
3 files changed, 64 insertions(+), 19 deletions(-)
diff --git a/client/gatt.c b/client/gatt.c
index d59d1ba1e..969a78be0 100644
--- a/client/gatt.c
+++ b/client/gatt.c
@@ -60,6 +60,7 @@ struct desc {
char *uuid;
char **flags;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
};
@@ -71,6 +72,7 @@ struct chrc {
bool notifying;
GList *descs;
int value_len;
+ unsigned int max_val_len;
uint8_t *value;
uint16_t mtu;
struct io *write_io;
@@ -612,7 +614,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
{
struct iovec iov;
struct write_attribute_data wd;
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -689,7 +691,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
static bool pipe_read(struct io *io, void *user_data)
{
struct chrc *chrc = user_data;
- uint8_t buf[512];
+ uint8_t buf[MAX_ATTR_VAL_LEN];
int fd = io_get_fd(io);
ssize_t bytes_read;
@@ -1611,7 +1613,8 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
}
-static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
+static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
+ int max_len)
{
DBusMessageIter array;
uint16_t offset = 0;
@@ -1628,6 +1631,9 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
if (parse_options(iter, &offset, NULL, NULL, NULL))
return -EINVAL;
+ if ((offset + read_len) > max_len)
+ return -EOVERFLOW;
+
if ((offset + read_len) > *len) {
*len = offset + read_len;
*value = g_realloc(*value, *len);
@@ -1645,6 +1651,7 @@ static void authorize_write_response(const char *input, void *user_data)
DBusMessageIter iter;
DBusMessage *reply;
char *err;
+ int errsv;
dbus_message_iter_init(pending_message, &iter);
@@ -1656,10 +1663,16 @@ static void authorize_write_response(const char *input, void *user_data)
chrc->authorized = true;
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
err = "org.bluez.Error.InvalidArguments";
goto error;
+ } else if (errsv == -EOVERFLOW) {
+ err = "org.bluez.Error.InvalidValueLength";
+
+ goto error;
}
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1685,6 +1698,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
struct chrc *chrc = user_data;
DBusMessageIter iter;
char *str;
+ int errsv;
dbus_message_iter_init(msg, &iter);
@@ -1706,10 +1720,15 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
return NULL;
}
- if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
+ errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
+ chrc->max_val_len);
+ if (errsv == -EINVAL) {
return g_dbus_create_error(msg,
- "org.bluez.Error.InvalidArguments",
- NULL);
+ "org.bluez.Error.InvalidArguments", NULL);
+ } else if (errsv == -EOVERFLOW) {
+ return g_dbus_create_error(msg,
+ "org.bluez.Error.InvalidValueLength", NULL);
+ }
bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
@@ -1881,9 +1900,9 @@ static const GDBusMethodTable chrc_methods[] = {
{ }
};
-static uint8_t *str2bytearray(char *arg, int *val_len)
+static uint8_t *str2bytearray(char *arg, int *val_len, unsigned int max_val_len)
{
- uint8_t value[512];
+ uint8_t value[MAX_ATTR_VAL_LEN];
char *entry;
unsigned int i;
@@ -1908,6 +1927,12 @@ static uint8_t *str2bytearray(char *arg, int *val_len)
value[i] = val;
}
+ if (i > max_val_len) {
+ bt_shell_printf("Value exceeds maximum length (%i)\n",
+ max_val_len);
+ return NULL;
+ }
+
*val_len = i;
return g_memdup(value, i);
@@ -1919,7 +1944,13 @@ static void chrc_set_value(const char *input, void *user_data)
g_free(chrc->value);
- chrc->value = str2bytearray((char *) input, &chrc->value_len);
+ chrc->value = str2bytearray((char *) input, &chrc->value_len,
+ chrc->max_val_len);
+
+ if (!chrc->value) {
+ print_chrc(chrc, COLORED_DEL);
+ chrc_unregister(chrc);
+ }
}
void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
@@ -1940,7 +1971,8 @@ void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
chrc->uuid = g_strdup(argv[1]);
chrc->path = g_strdup_printf("%s/chrc%p", service->path, chrc);
chrc->flags = g_strsplit(argv[2], ",", -1);
- chrc->authorization_req = argc > 3 ? true : false;
+ chrc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
+ chrc->authorization_req = argc > 4 ? true : false;
if (g_dbus_register_interface(conn, chrc->path, CHRC_INTERFACE,
chrc_methods, NULL, chrc_properties,
@@ -2037,7 +2069,8 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_init(msg, &iter);
- if (parse_value_arg(&iter, &desc->value, &desc->value_len))
+ if (parse_value_arg(&iter, &desc->value, &desc->value_len,
+ desc->max_val_len))
return g_dbus_create_error(msg,
"org.bluez.Error.InvalidArguments",
NULL);
@@ -2140,7 +2173,13 @@ static void desc_set_value(const char *input, void *user_data)
g_free(desc->value);
- desc->value = str2bytearray((char *) input, &desc->value_len);
+ desc->value = str2bytearray((char *) input, &desc->value_len,
+ desc->max_val_len);
+
+ if (!desc->value) {
+ print_desc(desc, COLORED_DEL);
+ desc_unregister(desc);
+ }
}
void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
@@ -2166,6 +2205,7 @@ void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
desc->uuid = g_strdup(argv[1]);
desc->path = g_strdup_printf("%s/desc%p", desc->chrc->path, desc);
desc->flags = g_strsplit(argv[2], ",", -1);
+ desc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
if (g_dbus_register_interface(conn, desc->path, DESC_INTERFACE,
desc_methods, NULL, desc_properties,
diff --git a/client/gatt.h b/client/gatt.h
index 957ae8003..189a0aa40 100644
--- a/client/gatt.h
+++ b/client/gatt.h
@@ -21,6 +21,10 @@
*
*/
+#define MAX_ATTR_VAL_LEN 512
+#define STR_(i) #i
+#define STR(i) STR_(i)
+
void gatt_add_service(GDBusProxy *proxy);
void gatt_remove_service(GDBusProxy *proxy);
diff --git a/client/main.c b/client/main.c
index dd85a1c85..60e0f10b0 100644
--- a/client/main.c
+++ b/client/main.c
@@ -2013,7 +2013,7 @@ static void cmd_register_characteristic(int argc, char *argv[])
if (check_default_ctrl() == FALSE)
return bt_shell_noninteractive_quit(EXIT_FAILURE);
- if (argc > 3 && strcmp(argv[3], "authorize")) {
+ if (argc > 4 && strcmp(argv[4], "authorize")) {
bt_shell_printf("Invalid authorize argument\n");
return bt_shell_noninteractive_quit(EXIT_FAILURE);
}
@@ -2437,14 +2437,15 @@ static const struct bt_shell_menu gatt_menu = {
{ "unregister-service", "<UUID/object>", cmd_unregister_service,
"Unregister application service" },
{ "register-characteristic", "<UUID> <Flags=read,write,notify...> "
- "[authorize]", cmd_register_characteristic,
- "Register application characteristic" },
+ "[max value len=0(default=" STR(MAX_ATTR_VAL_LEN) ")-"
+ STR(MAX_ATTR_VAL_LEN) "] [authorize]",
+ cmd_register_characteristic,
+ "Register application characteristic" },
{ "unregister-characteristic", "<UUID/object>",
cmd_unregister_characteristic,
"Unregister application characteristic" },
- { "register-descriptor", "<UUID> <Flags=read,write...>",
- cmd_register_descriptor,
- "Register application descriptor" },
+ { "register-descriptor", "<UUID> <Flags=read,write...> [max value len]",
+ cmd_register_descriptor, "Register application descriptor" },
{ "unregister-descriptor", "<UUID/object>",
cmd_unregister_descriptor,
"Unregister application descriptor" },
--
2.13.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH BlueZ] client: Add possiblity to define maximum attribute value length
2018-04-26 14:55 Grzegorz Kolodziejczyk
@ 2018-04-26 16:05 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2018-04-26 16:05 UTC (permalink / raw)
To: Grzegorz Kolodziejczyk; +Cc: linux-bluetooth
Hi Grzegorz,
On Thu, Apr 26, 2018 at 5:55 PM, Grzegorz Kolodziejczyk
<grzegorz.kolodziejczyk@codecoup.pl> wrote:
> This patch allows to define maximum value length for characteristic and
> descriptor value while registering.
> ---
> client/gatt.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++------------
> client/gatt.h | 4 ++++
> client/main.c | 13 ++++++------
> 3 files changed, 64 insertions(+), 19 deletions(-)
>
> diff --git a/client/gatt.c b/client/gatt.c
> index d59d1ba1e..969a78be0 100644
> --- a/client/gatt.c
> +++ b/client/gatt.c
> @@ -60,6 +60,7 @@ struct desc {
> char *uuid;
> char **flags;
> int value_len;
> + unsigned int max_val_len;
> uint8_t *value;
> };
>
> @@ -71,6 +72,7 @@ struct chrc {
> bool notifying;
> GList *descs;
> int value_len;
> + unsigned int max_val_len;
> uint8_t *value;
> uint16_t mtu;
> struct io *write_io;
> @@ -612,7 +614,7 @@ static void write_attribute(GDBusProxy *proxy, char *val_str, uint16_t offset)
> {
> struct iovec iov;
> struct write_attribute_data wd;
> - uint8_t value[512];
> + uint8_t value[MAX_ATTR_VAL_LEN];
> char *entry;
> unsigned int i;
>
> @@ -689,7 +691,7 @@ void gatt_write_attribute(GDBusProxy *proxy, int argc, char *argv[])
> static bool pipe_read(struct io *io, void *user_data)
> {
> struct chrc *chrc = user_data;
> - uint8_t buf[512];
> + uint8_t buf[MAX_ATTR_VAL_LEN];
> int fd = io_get_fd(io);
> ssize_t bytes_read;
>
> @@ -1611,7 +1613,8 @@ static DBusMessage *chrc_read_value(DBusConnection *conn, DBusMessage *msg,
> return read_value(msg, &chrc->value[offset], chrc->value_len - offset);
> }
>
> -static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
> +static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len,
> + int max_len)
> {
> DBusMessageIter array;
> uint16_t offset = 0;
> @@ -1628,6 +1631,9 @@ static int parse_value_arg(DBusMessageIter *iter, uint8_t **value, int *len)
> if (parse_options(iter, &offset, NULL, NULL, NULL))
> return -EINVAL;
>
> + if ((offset + read_len) > max_len)
> + return -EOVERFLOW;
> +
> if ((offset + read_len) > *len) {
> *len = offset + read_len;
> *value = g_realloc(*value, *len);
> @@ -1645,6 +1651,7 @@ static void authorize_write_response(const char *input, void *user_data)
> DBusMessageIter iter;
> DBusMessage *reply;
> char *err;
> + int errsv;
>
> dbus_message_iter_init(pending_message, &iter);
>
> @@ -1656,10 +1663,16 @@ static void authorize_write_response(const char *input, void *user_data)
>
> chrc->authorized = true;
>
> - if (parse_value_arg(&iter, &chrc->value, &chrc->value_len)) {
> + errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
> + chrc->max_val_len);
> + if (errsv == -EINVAL) {
> err = "org.bluez.Error.InvalidArguments";
>
> goto error;
> + } else if (errsv == -EOVERFLOW) {
> + err = "org.bluez.Error.InvalidValueLength";
> +
> + goto error;
> }
>
> bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
> @@ -1685,6 +1698,7 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> struct chrc *chrc = user_data;
> DBusMessageIter iter;
> char *str;
> + int errsv;
>
> dbus_message_iter_init(msg, &iter);
>
> @@ -1706,10 +1720,15 @@ static DBusMessage *chrc_write_value(DBusConnection *conn, DBusMessage *msg,
> return NULL;
> }
>
> - if (parse_value_arg(&iter, &chrc->value, &chrc->value_len))
> + errsv = parse_value_arg(&iter, &chrc->value, &chrc->value_len,
> + chrc->max_val_len);
> + if (errsv == -EINVAL) {
> return g_dbus_create_error(msg,
> - "org.bluez.Error.InvalidArguments",
> - NULL);
> + "org.bluez.Error.InvalidArguments", NULL);
> + } else if (errsv == -EOVERFLOW) {
> + return g_dbus_create_error(msg,
> + "org.bluez.Error.InvalidValueLength", NULL);
> + }
>
> bt_shell_printf("[" COLORED_CHG "] Attribute %s written" , chrc->path);
>
> @@ -1881,9 +1900,9 @@ static const GDBusMethodTable chrc_methods[] = {
> { }
> };
>
> -static uint8_t *str2bytearray(char *arg, int *val_len)
> +static uint8_t *str2bytearray(char *arg, int *val_len, unsigned int max_val_len)
> {
> - uint8_t value[512];
> + uint8_t value[MAX_ATTR_VAL_LEN];
> char *entry;
> unsigned int i;
>
> @@ -1908,6 +1927,12 @@ static uint8_t *str2bytearray(char *arg, int *val_len)
> value[i] = val;
> }
>
> + if (i > max_val_len) {
> + bt_shell_printf("Value exceeds maximum length (%i)\n",
> + max_val_len);
> + return NULL;
> + }
> +
> *val_len = i;
>
> return g_memdup(value, i);
> @@ -1919,7 +1944,13 @@ static void chrc_set_value(const char *input, void *user_data)
>
> g_free(chrc->value);
>
> - chrc->value = str2bytearray((char *) input, &chrc->value_len);
> + chrc->value = str2bytearray((char *) input, &chrc->value_len,
> + chrc->max_val_len);
> +
> + if (!chrc->value) {
> + print_chrc(chrc, COLORED_DEL);
> + chrc_unregister(chrc);
> + }
> }
>
> void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
> @@ -1940,7 +1971,8 @@ void gatt_register_chrc(DBusConnection *conn, GDBusProxy *proxy,
> chrc->uuid = g_strdup(argv[1]);
> chrc->path = g_strdup_printf("%s/chrc%p", service->path, chrc);
> chrc->flags = g_strsplit(argv[2], ",", -1);
> - chrc->authorization_req = argc > 3 ? true : false;
> + chrc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
> + chrc->authorization_req = argc > 4 ? true : false;
>
> if (g_dbus_register_interface(conn, chrc->path, CHRC_INTERFACE,
> chrc_methods, NULL, chrc_properties,
> @@ -2037,7 +2069,8 @@ static DBusMessage *desc_write_value(DBusConnection *conn, DBusMessage *msg,
>
> dbus_message_iter_init(msg, &iter);
>
> - if (parse_value_arg(&iter, &desc->value, &desc->value_len))
> + if (parse_value_arg(&iter, &desc->value, &desc->value_len,
> + desc->max_val_len))
> return g_dbus_create_error(msg,
> "org.bluez.Error.InvalidArguments",
> NULL);
> @@ -2140,7 +2173,13 @@ static void desc_set_value(const char *input, void *user_data)
>
> g_free(desc->value);
>
> - desc->value = str2bytearray((char *) input, &desc->value_len);
> + desc->value = str2bytearray((char *) input, &desc->value_len,
> + desc->max_val_len);
> +
> + if (!desc->value) {
> + print_desc(desc, COLORED_DEL);
> + desc_unregister(desc);
> + }
> }
>
> void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
> @@ -2166,6 +2205,7 @@ void gatt_register_desc(DBusConnection *conn, GDBusProxy *proxy,
> desc->uuid = g_strdup(argv[1]);
> desc->path = g_strdup_printf("%s/desc%p", desc->chrc->path, desc);
> desc->flags = g_strsplit(argv[2], ",", -1);
> + desc->max_val_len = argc > 3 ? atoi(argv[3]) : MAX_ATTR_VAL_LEN;
>
> if (g_dbus_register_interface(conn, desc->path, DESC_INTERFACE,
> desc_methods, NULL, desc_properties,
> diff --git a/client/gatt.h b/client/gatt.h
> index 957ae8003..189a0aa40 100644
> --- a/client/gatt.h
> +++ b/client/gatt.h
> @@ -21,6 +21,10 @@
> *
> */
>
> +#define MAX_ATTR_VAL_LEN 512
> +#define STR_(i) #i
> +#define STR(i) STR_(i)
> +
> void gatt_add_service(GDBusProxy *proxy);
> void gatt_remove_service(GDBusProxy *proxy);
>
> diff --git a/client/main.c b/client/main.c
> index dd85a1c85..60e0f10b0 100644
> --- a/client/main.c
> +++ b/client/main.c
> @@ -2013,7 +2013,7 @@ static void cmd_register_characteristic(int argc, char *argv[])
> if (check_default_ctrl() == FALSE)
> return bt_shell_noninteractive_quit(EXIT_FAILURE);
>
> - if (argc > 3 && strcmp(argv[3], "authorize")) {
> + if (argc > 4 && strcmp(argv[4], "authorize")) {
> bt_shell_printf("Invalid authorize argument\n");
> return bt_shell_noninteractive_quit(EXIT_FAILURE);
> }
> @@ -2437,14 +2437,15 @@ static const struct bt_shell_menu gatt_menu = {
> { "unregister-service", "<UUID/object>", cmd_unregister_service,
> "Unregister application service" },
> { "register-characteristic", "<UUID> <Flags=read,write,notify...> "
> - "[authorize]", cmd_register_characteristic,
> - "Register application characteristic" },
> + "[max value len=0(default=" STR(MAX_ATTR_VAL_LEN) ")-"
> + STR(MAX_ATTR_VAL_LEN) "] [authorize]",
> + cmd_register_characteristic,
> + "Register application characteristic" },
Id like to do it a little different, when we query to the user we can
actually determine how many arguments it has provided, or perhaps we
could ask the size first and then ask the value that way we don't have
to clutter the argument string.
> { "unregister-characteristic", "<UUID/object>",
> cmd_unregister_characteristic,
> "Unregister application characteristic" },
> - { "register-descriptor", "<UUID> <Flags=read,write...>",
> - cmd_register_descriptor,
> - "Register application descriptor" },
> + { "register-descriptor", "<UUID> <Flags=read,write...> [max value len]",
> + cmd_register_descriptor, "Register application descriptor" },
> { "unregister-descriptor", "<UUID/object>",
> cmd_unregister_descriptor,
> "Unregister application descriptor" },
> --
> 2.13.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-04-27 14:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-27 14:01 [PATCH BlueZ] client: Add possiblity to define maximum attribute value length Grzegorz Kolodziejczyk
2018-04-27 14:01 ` [PATCH BlueZ] client: Define maximum attribute value length as initial value Grzegorz Kolodziejczyk
2018-04-27 14:19 ` [PATCH BlueZ] client: Add possiblity to define maximum attribute value length Grzegorz Kołodziejczyk
-- strict thread matches above, loose matches on Subject: below --
2018-04-27 14:01 Grzegorz Kolodziejczyk
2018-04-26 14:55 Grzegorz Kolodziejczyk
2018-04-26 16:05 ` Luiz Augusto von Dentz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.