[rrd-developers] rrdtool: *** glibc detected *** free(): invalid pointer: 0x08056450 ***

[Bernhard Fischer]

On Wed, Jun 11, 2008 at 07:03:48PM +0200, Sebastian Harl wrote:
>Hi again,
>
>On Wed, Jun 11, 2008 at 10:50:17AM +0200, Sebastian Harl wrote:
>> On Sun, Nov 18, 2007 at 10:49:21PM +0100, Matej Kosik wrote:
>> > when I perform the following command
>> > 
>> > rrdtool graph /tmp/plot_1195420761938267.png --width 479 --height 71 --start 1195257600 --end 1195344000 -i -y 1:1 -l 0 -u 1 DEF:mystate=node_0.rrd:state:AVERAGE LINE2:mystate#FF0000
>> > 
>> > I get an error:
>> > 
>> > *** glibc detected *** free(): invalid pointer: 0x08056450 ***
>> > 
>> > and no graph is generated. The node_0.rrd file was created as follows:
>> > 
>> > 	rrdtool create /usr/local/src/nms/nms/priv/rrd/node_0.rrd -s 60000 DS:state:GAUGE:60:U:U RRA:AVERAGE:0.5:1:604800
>[...]
>>   start_offset = (long) (*start + *step - rra_start_time) / (long) *step;
>> 
>> I suspect that we're getting some kind of overflow here. Tobi do you have any
>> ideas about that?
>
>Okay, so here is the real issue:
>
>In this case a fairly large row count (604800) and a really unusually
>large step size (60000) was used. Now, the critical part of
>rrd_fetch_fn() is the calculation of rra_start_time (which in turn is
>then used to calculate start_offset):
>
>  rra_start_time = (rra_end_time
>                   - (*step * (rrd.rra_def[chosen_rra].row_cnt - 1)));
>
>step * (row_cnt - 1) equals 36287940000 which is way bigger than
>LONG_MAX on 32bit architectures (which usually equals 2147483647). So,
>we end up with an overflow of the "long int" value and rra_start_time

Didn't look, admittedly, but aren't they unsigned long int and could
eventually be unsigned long long int (which would potentially hurt
alot)?

I think that we should better avoid the trouble right on spot and try to
calculate the upcoming wrapping beforehand.

>containing some nonsense value.
>
>I'm currently unsure how to solve that properly. It might make sense to
>switch to some 64bit type here - that's what's required by the LFS

LFS itself optional, so no, not required.

We can test if it will wrap and should be (possibly) able to
precalculate it here instead of later on.