* Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f
@ 2012-10-17 12:49 Ludek Finstrle
2012-10-17 14:03 ` Vinicius Costa Gomes
2012-10-17 14:08 ` Anderson Lizardo
0 siblings, 2 replies; 6+ messages in thread
From: Ludek Finstrle @ 2012-10-17 12:49 UTC (permalink / raw)
To: linux-bluetooth
Hello,
I see memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f:
attrib: Fix not checking if att_data_list_alloc fails
It returns (in src/attrib-server.c) from functions when adl local variable is NULL but
it doesn't free local variables (read_by_group: group; read_by_type: type; find_info: info).
The patch is very easy but I can't test it with head so I don't want to send possible
crapy patch.
Luf
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f
2012-10-17 12:49 Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f Ludek Finstrle
@ 2012-10-17 14:03 ` Vinicius Costa Gomes
2012-10-17 14:08 ` Anderson Lizardo
1 sibling, 0 replies; 6+ messages in thread
From: Vinicius Costa Gomes @ 2012-10-17 14:03 UTC (permalink / raw)
To: Ludek Finstrle; +Cc: linux-bluetooth
Hi Luf,
On 14:49 Wed 17 Oct, Ludek Finstrle wrote:
> Hello,
>
> I see memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f:
> attrib: Fix not checking if att_data_list_alloc fails
>
> It returns (in src/attrib-server.c) from functions when adl local variable is NULL but
> it doesn't free local variables (read_by_group: group; read_by_type: type; find_info: info).
Thanks a lot for the report.
Ugh, I will fix it.
>
> The patch is very easy but I can't test it with head so I don't want to send possible
> crapy patch.
>
> Luf
> --
> 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
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f
2012-10-17 12:49 Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f Ludek Finstrle
2012-10-17 14:03 ` Vinicius Costa Gomes
@ 2012-10-17 14:08 ` Anderson Lizardo
2012-10-18 8:39 ` [RFC] attrib: Fix memleak if att_data_list_alloc fails Ludek Finstrle
1 sibling, 1 reply; 6+ messages in thread
From: Anderson Lizardo @ 2012-10-17 14:08 UTC (permalink / raw)
To: Ludek Finstrle; +Cc: linux-bluetooth
Hi Ludek,
On Wed, Oct 17, 2012 at 9:49 AM, Ludek Finstrle <luf@pzkagis.cz> wrote:
> Hello,
>
> I see memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f:
> attrib: Fix not checking if att_data_list_alloc fails
>
> It returns (in src/attrib-server.c) from functions when adl local variable is NULL but
> it doesn't free local variables (read_by_group: group; read_by_type: type; find_info: info).
>
> The patch is very easy but I can't test it with head so I don't want to send possible
> crapy patch.
Please feel free to send it marked as RFC on the subject (e.g. "[RFC]
bla bla") and rebased against master, so we can review the fix.
Best Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC] attrib: Fix memleak if att_data_list_alloc fails
2012-10-17 14:08 ` Anderson Lizardo
@ 2012-10-18 8:39 ` Ludek Finstrle
2012-10-18 23:20 ` Vinicius Costa Gomes
2012-10-19 7:42 ` Johan Hedberg
0 siblings, 2 replies; 6+ messages in thread
From: Ludek Finstrle @ 2012-10-18 8:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Ludek Finstrle
Fix for memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f.
I tested it with older bluez and rebase for git. Please review it.
---
src/attrib-server.c | 12 +++++++++---
1 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/src/attrib-server.c b/src/attrib-server.c
index ea27b13..4489edc 100644
--- a/src/attrib-server.c
+++ b/src/attrib-server.c
@@ -490,9 +490,11 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
length = g_slist_length(groups);
adl = att_data_list_alloc(length, last_size + 4);
- if (adl == NULL)
+ if (adl == NULL) {
+ g_slist_free_full(groups, g_free);
return enc_error_resp(ATT_OP_READ_BY_GROUP_REQ, start,
ATT_ECODE_UNLIKELY, pdu, len);
+ }
for (i = 0, l = groups; l; l = l->next, i++) {
uint8_t *value;
@@ -577,9 +579,11 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
length += 2;
adl = att_data_list_alloc(num, length);
- if (adl == NULL)
+ if (adl == NULL) {
+ g_slist_free(types);
return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ, start,
ATT_ECODE_UNLIKELY, pdu, len);
+ }
for (i = 0, l = types; l; i++, l = l->next) {
uint8_t *value;
@@ -655,9 +659,11 @@ static uint16_t find_info(struct gatt_channel *channel, uint16_t start,
}
adl = att_data_list_alloc(num, length + 2);
- if (adl == NULL)
+ if (adl == NULL) {
+ g_slist_free(info);
return enc_error_resp(ATT_OP_FIND_INFO_REQ, start,
ATT_ECODE_UNLIKELY, pdu, len);
+ }
for (i = 0, l = info; l; i++, l = l->next) {
uint8_t *value;
--
1.7.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC] attrib: Fix memleak if att_data_list_alloc fails
2012-10-18 8:39 ` [RFC] attrib: Fix memleak if att_data_list_alloc fails Ludek Finstrle
@ 2012-10-18 23:20 ` Vinicius Costa Gomes
2012-10-19 7:42 ` Johan Hedberg
1 sibling, 0 replies; 6+ messages in thread
From: Vinicius Costa Gomes @ 2012-10-18 23:20 UTC (permalink / raw)
To: Ludek Finstrle; +Cc: linux-bluetooth
Hi,
On 10:39 Thu 18 Oct, Ludek Finstrle wrote:
> Fix for memory leak which was introduced in commit f8619bef3406a2134082dc41c208105fe028c09f.
> I tested it with older bluez and rebase for git. Please review it.
Patch looks good. I would just re-send it without the last line in the
commit message.
But anyway, here's my Ack.
>
> ---
> src/attrib-server.c | 12 +++++++++---
> 1 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/src/attrib-server.c b/src/attrib-server.c
> index ea27b13..4489edc 100644
> --- a/src/attrib-server.c
> +++ b/src/attrib-server.c
> @@ -490,9 +490,11 @@ static uint16_t read_by_group(struct gatt_channel *channel, uint16_t start,
> length = g_slist_length(groups);
>
> adl = att_data_list_alloc(length, last_size + 4);
> - if (adl == NULL)
> + if (adl == NULL) {
> + g_slist_free_full(groups, g_free);
> return enc_error_resp(ATT_OP_READ_BY_GROUP_REQ, start,
> ATT_ECODE_UNLIKELY, pdu, len);
> + }
>
> for (i = 0, l = groups; l; l = l->next, i++) {
> uint8_t *value;
> @@ -577,9 +579,11 @@ static uint16_t read_by_type(struct gatt_channel *channel, uint16_t start,
> length += 2;
>
> adl = att_data_list_alloc(num, length);
> - if (adl == NULL)
> + if (adl == NULL) {
> + g_slist_free(types);
> return enc_error_resp(ATT_OP_READ_BY_TYPE_REQ, start,
> ATT_ECODE_UNLIKELY, pdu, len);
> + }
>
> for (i = 0, l = types; l; i++, l = l->next) {
> uint8_t *value;
> @@ -655,9 +659,11 @@ static uint16_t find_info(struct gatt_channel *channel, uint16_t start,
> }
>
> adl = att_data_list_alloc(num, length + 2);
> - if (adl == NULL)
> + if (adl == NULL) {
> + g_slist_free(info);
> return enc_error_resp(ATT_OP_FIND_INFO_REQ, start,
> ATT_ECODE_UNLIKELY, pdu, len);
> + }
>
> for (i = 0, l = info; l; i++, l = l->next) {
> uint8_t *value;
> --
> 1.7.1
>
> --
> 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
Cheers,
--
Vinicius
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC] attrib: Fix memleak if att_data_list_alloc fails
2012-10-18 8:39 ` [RFC] attrib: Fix memleak if att_data_list_alloc fails Ludek Finstrle
2012-10-18 23:20 ` Vinicius Costa Gomes
@ 2012-10-19 7:42 ` Johan Hedberg
1 sibling, 0 replies; 6+ messages in thread
From: Johan Hedberg @ 2012-10-19 7:42 UTC (permalink / raw)
To: Ludek Finstrle; +Cc: linux-bluetooth
Hi Ludek,
On Thu, Oct 18, 2012, Ludek Finstrle wrote:
> Fix for memory leak which was introduced in commit
> f8619bef3406a2134082dc41c208105fe028c09f.
> I tested it with older bluez and rebase for git. Please review it.
> ---
> src/attrib-server.c | 12 +++++++++---
> 1 files changed, 9 insertions(+), 3 deletions(-)
Applied (with the last commit message line removed as suggested by
Vinicius). Thanks.
Johan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-10-19 7:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-17 12:49 Memory leak introduced in commit f8619bef3406a2134082dc41c208105fe028c09f Ludek Finstrle
2012-10-17 14:03 ` Vinicius Costa Gomes
2012-10-17 14:08 ` Anderson Lizardo
2012-10-18 8:39 ` [RFC] attrib: Fix memleak if att_data_list_alloc fails Ludek Finstrle
2012-10-18 23:20 ` Vinicius Costa Gomes
2012-10-19 7:42 ` Johan Hedberg
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).