* 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).