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

[Bernhard Fischer]

On Wed, Jun 11, 2008 at 11:19:21AM +0200, Sebastian Harl wrote:
>tags 451852 + patch
>thanks
>
>Hi,
>
>On Wed, Jun 11, 2008 at 10:50:17AM +0200, Sebastian Harl wrote:
>> start_offset is calculated in line 348:
>> 
>>   start_offset = (long) (*start + *step - rra_start_time) / (long) *step;
>> 
>> I suspect that we're getting some kind of overflow here.
>
>Okay, I think I found the problem: The variables start and
>rra_start_time are of type "time_t" which does not seem to be large
>enough to store the result of that calculation and thus overflows.
>
>The attached patch seems to fix the problem for me. Possibly, some other
>parts of the code need similar fixes as well but I don't have to the
>time to take a closer look at that right now.
>
>Cheers,
>Sebastian
>
>-- 
>Sebastian "tokkee" Harl +++ GnuPG-ID: 0x8501C7FC +++ http://tokkee.org/
>
>Those who would give up Essential Liberty to purchase a little Temporary
>Safety, deserve neither Liberty nor Safety.         -- Benjamin Franklin
>

>From 5ee3d7e6d567aa0e1ce770bb244ae1dc7fb80d33 Mon Sep 17 00:00:00 2001
>From: Sebastian Harl <sh at tokkee.org>
>Date: Wed, 11 Jun 2008 11:04:13 +0200
>Subject: [PATCH] Prevent a overflow when calculating offsets in rrd_fetch_fn().
>
>Some variables of type time_t are used for calculating those values. If the
>RRD step size exceeds a certain value (3550 in my case), the result got too
>large to be stored in a time_t variable and thus overflowed. A cast to type
>long helped to solve this problem.
>---
> program/src/rrd_fetch.c |    5 +++--
> 1 files changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/program/src/rrd_fetch.c b/program/src/rrd_fetch.c
>index 4ea2eb1..10a80d8 100644
>--- a/program/src/rrd_fetch.c
>+++ b/program/src/rrd_fetch.c
>@@ -345,8 +345,9 @@ int rrd_fetch_fn(
>     rra_start_time = (rra_end_time
>                       - (*step * (rrd.rra_def[chosen_rra].row_cnt - 1)));
>     /* here's an error by one if we don't be careful */
>-    start_offset = (long) (*start + *step - rra_start_time) / (long) *step;
>-    end_offset = (long) (rra_end_time - *end) / (long) *step;
>+    start_offset = (long) *start + *step - (long) rra_start_time
>+                   / (long) *step;

careful what you're dividing!

>+    end_offset = (long) rra_end_time - (long) *end / (long) *step;

ditto.