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 nt...@debian.org
>From c0ac2ba5afeedd535367d78bf39cbe58e968a345 Mon Sep 17 00:00:00 2001 From: Niko Tyni <nt...@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