Sorry, how do I create a JIRA issue? 27.05.2015 16:48, Alan Conway пишет: > On Tue, 2015-05-26 at 23:25 +0300, Michael Ivanov wrote: >> Greetings, >> >> I have observed that pn_data_grow() function looses half of the available >> data capacity. >> The following happens: when data overflows, pn_data_grow is invoked. It >> increases >> data capacity 2 times and reallocates nodes array. Data capacity is >> represented as >> uint16_t type and so when capacity reaches 32768 items, the result of >> multiplication by 2 >> becomes 0. This makes realloc return null and crashes the program. >> >> To alleviate the problem with large messages I changed the function as >> follows: >> >> --- qpid-proton-0.9/proton-c/src/codec/codec.c 2015-03-31 >> 12:07:22.000000000 +0300 >> +++ qpid-proton-0.9.fix/proton-c/src/codec/codec.c 2015-05-26 >> 21:18:55.801632941 +0300 >> @@ -417,8 +417,21 @@ void pn_data_clear(pn_data_t *data) >> >> int pn_data_grow(pn_data_t *data) >> { >> - data->capacity = 2*(data->capacity ? data->capacity : 2); >> - data->nodes = (pni_node_t *) realloc(data->nodes, data->capacity * >> sizeof(pni_node_t)); >> + size_t s = data->capacity; >> + >> + if (s < 0x7fff) >> + s = 2 * (s? s : 2); >> + else if (s < 0xffff - 1024) >> + s += 1024; >> + else if (s != 0xffff) >> + s = 0xffff; >> + else { >> + pn_logf("Data node %p overflow", data); >> + abort(); >> + } >> + >> + data->nodes = (pni_node_t *) realloc(data->nodes, s * sizeof(pni_node_t)); >> + data->capacity = s; >> return 0; >> >> This allows to use capacities in 0x8000 ... 0xffff range and is supposed to >> report >> data overflow. > > > Good catch, can you please create a JIRA with the patch attached so you > can get proper credit for it (for copyright reasons I can't take the > patch direct from email.) You can assign it to me, I'll apply it right > away. > > I would suggest a couple of changes: > > 1. Define an explicit PNI_NID_MAX in data.h right beside typedef > pni_nid_t rather than using literal 0xffff etc. in case someone changes > the definition of pni_nid_t at some point. > > 2. Use a simple double or max rather than adding 1k after 0x7fff. We > need to fix the algorithm but I don't think we need to add new > complexity unless there's a separate justification for that. > > 3. If we fail to increase (i.e. we're already at PNI_NID_MAX) return > PN_OVERFLOW rather than aborting. There is only one function > (pn_data_new) that calls pn_data_grow, make it check the return value > and return NULL on error. That won't make exiting code worse (it will > still crash on overrun) but will give well written code that checks > error values the choice. > > Thanks for this! > Alan. >
-- \ / | | (OvO) | Михаил Иванов | (^^^) | Тел.: +7(911) 223-1300 | \^/ | E-mail: iv...@isle.spb.ru | ^ ^ | |