client: Fix memory leak connected to params

PATCH In review Bartosz Szatkowski 2 participants 4 messages Nov 21, 2011 Bluetooth drivers

Share & tools

Bartosz Szatkowski

author

2011-11-21 08:32 UTC #0 lore raw

PATCH 1/2 client: Fix memory leak connected to params

obc_transfer_params (created in obc_session_get) are reused to store params received in the response. But actual parameter data were not freed before g_memdup in get_buf_xfer_progress.

---
 client/transfer.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

client/transfer.c +6 -1

index b6994d1..334d8d4 100644
@@ -533,11 +533,16 @@ int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
 		g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
 						strlen(transfer->type) + 1);
 
-	if (transfer->params != NULL)
+	if (transfer->params != NULL) {
 		g_obex_packet_add_bytes(req, G_OBEX_HDR_APPARAM,
 						transfer->params->data,
 						transfer->params->size);
 
+		g_free(transfer->params->data);
+		transfer->params->size = 0;
+		transfer->params->data = NULL;
+	}
+
 	if (rsp_cb)
 		transfer->xfer = g_obex_send_req(obex, req, -1, rsp_cb,
 							transfer, &err);

Signature · 1 line

1.7.4.1

Bartosz Szatkowski

author

2011-11-21 08:32 UTC #1 lore raw

PATCH 2/2 client: Fix invalid write in get_buf_xfer_progress

Segmentation fault occurred when there were no (NULL) params in obc_session_get, but actual params were returned in response, as corresponding structure is reused - but not created in the first place.

---
 client/session.c  |    2 +-
 client/transfer.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

client/session.c +1 -1

index f288ecd..e51faf1 100644
@@ -1231,8 +1231,8 @@ int obc_session_get(struct obc_session *session, const char *type,
 	if (session->obex == NULL)
 		return -ENOTCONN;
 
+	params = g_new0(struct obc_transfer_params, 1);
 	if (apparam != NULL) {
-		params = g_new0(struct obc_transfer_params, 1);
 		params->data = g_new(guint8, apparam_size);
 		memcpy(params->data, apparam, apparam_size);
 		params->size = apparam_size;

client/transfer.c +1 -1

index 334d8d4..0a44af5 100644
@@ -533,7 +533,7 @@ int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
 		g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
 						strlen(transfer->type) + 1);
 
-	if (transfer->params != NULL) {
+	if (transfer->params != NULL && transfer->params->size != 0) {
 		g_obex_packet_add_bytes(req, G_OBEX_HDR_APPARAM,
 						transfer->params->data,
 						transfer->params->size);

Signature · 1 line

1.7.4.1

Luiz Augusto von Dentz

maintainer

2011-11-21 09:53 UTC #2 lore raw

Re: [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress

Hi Bartosz,

Quoted 38 lines · Bartosz Szatkowski

 2 files changed, 2 insertions(+), 2 deletions(-)

client/session.c +1 -1

index f288ecd..e51faf1 100644
@@ -1231,8 +1231,8 @@ int obc_session_get(struct obc_session *session, const char *type,
        if (session->obex == NULL)
                return -ENOTCONN;
 
+       params = g_new0(struct obc_transfer_params, 1);
        if (apparam != NULL) {
-               params = g_new0(struct obc_transfer_params, 1);
                params->data = g_new(guint8, apparam_size);
                memcpy(params->data, apparam, apparam_size);
                params->size = apparam_size;

client/transfer.c +1 -2

index 334d8d4..0a44af5 100644
@@ -533,7 +533,7 @@ int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
                g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
                                                strlen(transfer->type) + 1);
 
-       if (transfer->params != NULL) {
+       if (transfer->params != NULL && transfer->params->size != 0) {
                g_obex_packet_add_bytes(req, G_OBEX_HDR_APPARAM,
                                                transfer->params->data,
                                                transfer->params->size);
--

1.7.4.1

Quoted 1 line

--

Ack.

Signature · 1 line

Luiz Augusto von Dentz

Luiz Augusto von Dentz

maintainer

2011-11-21 09:50 UTC #3 lore raw

In reply to Bartosz Szatkowski

Hi Bartosz,

Quoted 32 lines · Bartosz Szatkowski

obc_transfer_params (created in obc_session_get) are reused to store params received in the response. But actual parameter data were not freed before g_memdup in get_buf_xfer_progress.
---
client/transfer.c | 7 ++++++-

 1 files changed, 6 insertions(+), 1 deletions(-)

client/transfer.c +6 -2

index b6994d1..334d8d4 100644
@@ -533,11 +533,16 @@ int obc_transfer_get(struct obc_transfer *transfer, transfer_callback_t func,
                g_obex_packet_add_bytes(req, G_OBEX_HDR_TYPE, transfer->type,
                                                strlen(transfer->type) + 1);
 
-       if (transfer->params != NULL)
+       if (transfer->params != NULL) {
                g_obex_packet_add_bytes(req, G_OBEX_HDR_APPARAM,
                                                transfer->params->data,
                                                transfer->params->size);
 
+               g_free(transfer->params->data);
+               transfer->params->size = 0;
+               transfer->params->data = NULL;
+       }
+
        if (rsp_cb)
                transfer->xfer = g_obex_send_req(obex, req, -1, rsp_cb,
                                                        transfer, &err);
--

1.7.4.1

Quoted 1 line

--

I would like it better if do this on get_buf_xfer_progress, so whenever we are going to overwrite it we free it before.

Signature · 1 line

Luiz Augusto von Dentz