* [PATCH obexd 1/2] client: Fix memory leak connected to params
@ 2011-11-21 8:32 Bartosz Szatkowski
2011-11-21 8:32 ` [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress Bartosz Szatkowski
2011-11-21 9:50 ` [PATCH obexd 1/2] client: Fix memory leak connected to params Luiz Augusto von Dentz
0 siblings, 2 replies; 7+ messages in thread
From: Bartosz Szatkowski @ 2011-11-21 8:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: 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(-)
diff --git a/client/transfer.c b/client/transfer.c
index b6994d1..334d8d4 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress
2011-11-21 8:32 [PATCH obexd 1/2] client: Fix memory leak connected to params Bartosz Szatkowski
@ 2011-11-21 8:32 ` Bartosz Szatkowski
2011-11-21 9:53 ` Luiz Augusto von Dentz
2011-11-21 9:50 ` [PATCH obexd 1/2] client: Fix memory leak connected to params Luiz Augusto von Dentz
1 sibling, 1 reply; 7+ messages in thread
From: Bartosz Szatkowski @ 2011-11-21 8:32 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
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(-)
diff --git a/client/session.c b/client/session.c
index f288ecd..e51faf1 100644
--- a/client/session.c
+++ b/client/session.c
@@ -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;
diff --git a/client/transfer.c b/client/transfer.c
index 334d8d4..0a44af5 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH obexd 1/2] client: Fix memory leak connected to params
2011-11-21 8:32 [PATCH obexd 1/2] client: Fix memory leak connected to params Bartosz Szatkowski
2011-11-21 8:32 ` [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress Bartosz Szatkowski
@ 2011-11-21 9:50 ` Luiz Augusto von Dentz
1 sibling, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2011-11-21 9:50 UTC (permalink / raw)
To: Bartosz Szatkowski; +Cc: linux-bluetooth
Hi Bartosz,
On Mon, Nov 21, 2011 at 10:32 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> 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(-)
>
> diff --git a/client/transfer.c b/client/transfer.c
> index b6994d1..334d8d4 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -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
>
> --
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.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress
2011-11-21 8:32 ` [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress Bartosz Szatkowski
@ 2011-11-21 9:53 ` Luiz Augusto von Dentz
0 siblings, 0 replies; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2011-11-21 9:53 UTC (permalink / raw)
To: Bartosz Szatkowski; +Cc: linux-bluetooth
Hi Bartosz,
On Mon, Nov 21, 2011 at 10:32 AM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> 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(-)
>
> diff --git a/client/session.c b/client/session.c
> index f288ecd..e51faf1 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -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;
> diff --git a/client/transfer.c b/client/transfer.c
> index 334d8d4..0a44af5 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -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
>
> --
Ack.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress
2011-11-21 10:36 Bartosz Szatkowski
@ 2011-11-21 10:36 ` Bartosz Szatkowski
2011-11-21 11:01 ` Luiz Augusto von Dentz
0 siblings, 1 reply; 7+ messages in thread
From: Bartosz Szatkowski @ 2011-11-21 10:36 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Bartosz Szatkowski
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(-)
diff --git a/client/session.c b/client/session.c
index f288ecd..e51faf1 100644
--- a/client/session.c
+++ b/client/session.c
@@ -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;
diff --git a/client/transfer.c b/client/transfer.c
index e072a42..8340cc3 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -536,7 +536,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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress
2011-11-21 10:36 ` [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress Bartosz Szatkowski
@ 2011-11-21 11:01 ` Luiz Augusto von Dentz
2011-11-21 11:32 ` Bartosz Szatkowski
0 siblings, 1 reply; 7+ messages in thread
From: Luiz Augusto von Dentz @ 2011-11-21 11:01 UTC (permalink / raw)
To: Bartosz Szatkowski; +Cc: linux-bluetooth
Hi Bartosz,
On Mon, Nov 21, 2011 at 12:36 PM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
> 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(-)
>
> diff --git a/client/session.c b/client/session.c
> index f288ecd..e51faf1 100644
> --- a/client/session.c
> +++ b/client/session.c
> @@ -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;
> diff --git a/client/transfer.c b/client/transfer.c
> index e072a42..8340cc3 100644
> --- a/client/transfer.c
> +++ b/client/transfer.c
> @@ -536,7 +536,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
>
After some offline discussion with Johan we think it is better to have
this allocation completely on demand, so it means we should not
allocate it before hand if the request doesn't have any apparam and
then if the response has it then we allocate on get_buf_xfer_progress.
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress
2011-11-21 11:01 ` Luiz Augusto von Dentz
@ 2011-11-21 11:32 ` Bartosz Szatkowski
0 siblings, 0 replies; 7+ messages in thread
From: Bartosz Szatkowski @ 2011-11-21 11:32 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
On Mon, Nov 21, 2011 at 12:01 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi Bartosz,
>
> On Mon, Nov 21, 2011 at 12:36 PM, Bartosz Szatkowski <bulislaw@linux.com> wrote:
>> 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(-)
>>
>> diff --git a/client/session.c b/client/session.c
>> index f288ecd..e51faf1 100644
>> --- a/client/session.c
>> +++ b/client/session.c
>> @@ -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;
>> diff --git a/client/transfer.c b/client/transfer.c
>> index e072a42..8340cc3 100644
>> --- a/client/transfer.c
>> +++ b/client/transfer.c
>> @@ -536,7 +536,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
>>
>
> After some offline discussion with Johan we think it is better to have
> this allocation completely on demand, so it means we should not
> allocate it before hand if the request doesn't have any apparam and
> then if the response has it then we allocate on get_buf_xfer_progress.
>
> --
> Luiz Augusto von Dentz
>
Ok,
I'll send patch later this day.
--
Pozdrowienia - Cheers,
Bartosz Szatkowski
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-11-21 11:32 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-21 8:32 [PATCH obexd 1/2] client: Fix memory leak connected to params Bartosz Szatkowski
2011-11-21 8:32 ` [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress Bartosz Szatkowski
2011-11-21 9:53 ` Luiz Augusto von Dentz
2011-11-21 9:50 ` [PATCH obexd 1/2] client: Fix memory leak connected to params Luiz Augusto von Dentz
-- strict thread matches above, loose matches on Subject: below --
2011-11-21 10:36 Bartosz Szatkowski
2011-11-21 10:36 ` [PATCH obexd 2/2] client: Fix invalid write in get_buf_xfer_progress Bartosz Szatkowski
2011-11-21 11:01 ` Luiz Augusto von Dentz
2011-11-21 11:32 ` Bartosz Szatkowski
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).