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 | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/client/transfer.c b/client/transfer.c
index b6994d1..e072a42 100644
--- a/client/transfer.c
+++ b/client/transfer.c
@@ -356,6 +356,9 @@ static void get_buf_xfer_progress(GObex *obex, GError *err, GObexPacket *rsp,
if (hdr) {
g_obex_header_get_bytes(hdr, &buf, &len);
if (len != 0) {
+ if (transfer->params->data != NULL)
+ g_free(transfer->params->data);
+
transfer->params->data = g_memdup(buf, len);
transfer->params->size = len;
}
--
1.7.4.1
On Mon, Nov 21, 2011 at 12:01 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> Hi Bartosz,
>
> On Mon, Nov 21, 2011 at 12:36 PM, Bartosz Szatkowski <[email protected]> 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
Hi Bartosz,
On Mon, Nov 21, 2011 at 12:36 PM, Bartosz Szatkowski <[email protected]> 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
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
Hi Bartosz,
On Mon, Nov 21, 2011 at 10:32 AM, Bartosz Szatkowski <[email protected]> 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
Hi Bartosz,
On Mon, Nov 21, 2011 at 10:32 AM, Bartosz Szatkowski <[email protected]> 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