Re: [PATCH BlueZ] Fix allocation of attribute values

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Zitat von Anderson Lizardo <anderson.lizardo@xxxxxxxxxxxxx>:

From: Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxxxxxx>

Now that pointers to attribute are passed as reference to functions
we cannot have they change during run time, what g_try_realloc()
could do.
---
 attrib/att.h        |    2 +-
 src/attrib-server.c |   33 ++++++++++++++++++++++++---------
 2 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/attrib/att.h b/attrib/att.h
index 3b8c098..9db8065 100644
--- a/attrib/att.h
+++ b/attrib/att.h
@@ -130,7 +130,7 @@ struct attribute {
 	uint8_t (*write_cb)(struct attribute *a, gpointer user_data);
 	gpointer cb_user_data;
 	int len;
-	uint8_t data[0];
+	uint8_t *data;
 };

 struct att_data_list {
diff --git a/src/attrib-server.c b/src/attrib-server.c
index 0bda1e3..5c86085 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -270,7 +270,9 @@ static struct attribute *client_cfg_attribute(struct gatt_channel *channel,

 		/* Create a private copy of the Client Characteristic
 		 * Configuration attribute */
-		a = g_malloc0(sizeof(*a) + vlen);
+		a = g_new0(struct attribute, 1);
+		a->data = g_malloc0(vlen);

Here you assign the pointer...

+
 		memcpy(a, orig_attr, sizeof(*a));

...and here your overwrite it.
Hint: if that is written as
   *a = *orig_attr;
you get a compile-time type check for free.

 		memcpy(a->data, value, vlen);

And here you do...hmm...hard to tell since the pointer now shows to...where?
Even if that pointer is valid and is assigned the right size memory, you just leaked memory.

@@ -1242,7 +1252,9 @@ struct attribute *attrib_db_add(uint16_t handle, bt_uuid_t *uuid, int read_reqs,
 	if (g_slist_find_custom(database, GUINT_TO_POINTER(h), handle_cmp))
 		return NULL;

-	a = g_malloc0(sizeof(struct attribute) + len);
+	a = g_new0(struct attribute, 1);
+	a->data = g_malloc0(len);
+
 	a->handle = handle;
 	memcpy(&a->uuid, uuid, sizeof(bt_uuid_t));

This could also use assignement-instead-of-memcpy but that's out-of-scope of this patch?

 	a->read_reqs = read_reqs;
@@ -1268,21 +1280,23 @@ int attrib_db_update(uint16_t handle, bt_uuid_t *uuid, const uint8_t *value,
 	if (!l)
 		return -ENOENT;

-	a = g_try_realloc(l->data, sizeof(struct attribute) + len);
-	if (a == NULL)
+	a = l->data;
+
+	a->data = g_try_realloc(a->data, len);
+	if (a->data == NULL)
 		return -ENOMEM;

-	l->data = a;
-	if (uuid != NULL)
-		memcpy(&a->uuid, uuid, sizeof(bt_uuid_t));
 	a->len = len;
 	memcpy(a->data, value, len);

-	attrib_notify_clients(a);
+	if (uuid != NULL)
+		memcpy(&a->uuid, uuid, sizeof(bt_uuid_t));

But here you could write: a->uuid = *uuid;

HS


--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux