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: [email protected] |
^ ^ | |
