[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#805391: src:rrdtool: FTBFS on mips: `FAIL: tune2`



Control: tag -1 patch

On Sat, Dec 19, 2015 at 02:45:03PM +0100, Aurelien Jarno wrote:
> On 2015-12-19 15:03, Niko Tyni wrote:

> > It's a latent bug in rrdtool that triggered with libc6 2.19-20 on mips.

> > The problem is that rrd_write() ends up calling memcpy(3) on overlapping
> > memory areas, which is explicitly prohibited in its documentation. With
> > libc6 2.19-20 on mips, this started zeroing out part of the areas under
> > some conditions.
 
> The change is likely due to the switch to the MIPS32R2 ISA on mips.
> A different code is used, so with a different undefined behaviour.

Right, thanks.

> Can't you replace memcpy calls with overlapping areas with a memmove
> instead? Even if the testsuite looks fine you will definitely have some
> corruptions with memcpy and overlapping area on other architectures too.

Sure, that works and the memmove() overhead is presumably negligible
with mmap'ed disk IO. Patch attached, I've tested that the package
builds on mips and amd64 with this and forwarded it upstream at

  https://github.com/oetiker/rrdtool-1.x/pull/693

Jean-Michel: this is somewhat urgent given it blocks the Perl 5.22
transition, so please let us know if you'd like an NMU.
-- 
Niko Tyni   ntyni@debian.org
>From c0ac2ba5afeedd535367d78bf39cbe58e968a345 Mon Sep 17 00:00:00 2001
From: Niko Tyni <ntyni@debian.org>
Date: Sun, 20 Dec 2015 09:49:14 +0200
Subject: [PATCH] Use memmove instead of memcpy in rrd_write() to fix undefined
 behaviour

At least rrdtune ends up calling rrd_write() with the same memory
area for the source and the destination, causing undefined behaviour
that has been observed to actually break on the mips architecture.

Bug-Debian: https://bugs.debian.org/805391
Bug: https://github.com/oetiker/rrdtool-1.x/issues/688
---
 src/rrd_open.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/rrd_open.c b/src/rrd_open.c
index b4e151e..9e05164 100644
--- a/src/rrd_open.c
+++ b/src/rrd_open.c
@@ -808,7 +808,8 @@ ssize_t rrd_write(
         rrd_set_error("attempting to write beyond end of file (%ld + %ld > %ld)",rrd_file->pos, count, old_size);
         return -1;
     }
-    memcpy(rrd_simple_file->file_start + rrd_file->pos, buf, count);
+    /* can't use memcpy since the areas overlap when tuning */
+    memmove(rrd_simple_file->file_start + rrd_file->pos, buf, count);
     rrd_file->pos += count;
     return count;       /* mimmic write() semantics */
 #else
-- 
2.6.4


Reply to: