From: Jim Meyering Date: Tue, 16 Oct 2012 15:43:49 +0000 (+0200) Subject: cp: avoid data-corrupting free-memory-read X-Git-Tag: v8.20~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=64aef5fb9afecc023a6e719da161dbbf450908b8;p=thirdparty%2Fcoreutils.git cp: avoid data-corrupting free-memory-read * src/extent-scan.c (extent_scan_read): Reset our last_ei pointer whenever the parent buffer might have just been freed. * tests/cp/fiemap-extent-FMR.sh: New test. * tests/local.mk (all_tests): Add it. * NEWS (Bug fixes): Mention it. Reported by Mike Gerth in http://bugs.gnu.org/12656, and with help from Alan Curry. Bug introduced in commit v8.10-60-g18f5a85. --- diff --git a/NEWS b/NEWS index aff5bf18e9..0fc2a94d1b 100644 --- a/NEWS +++ b/NEWS @@ -12,6 +12,11 @@ GNU coreutils NEWS -*- outline -*- ** Bug fixes + cp could read from freed memory and could even make corrupt copies. + This could happen with a very fragmented and sparse input file, + on GNU/Linux file systems supporting fiemap extent scanning. + [bug introduced in coreutils-8.11] + cp --no-preserve=mode now no longer preserves the original file's permissions but correctly sets mode specified by 0666 & ~umask diff --git a/src/extent-scan.c b/src/extent-scan.c index 0c25c57052..f962298556 100644 --- a/src/extent-scan.c +++ b/src/extent-scan.c @@ -89,7 +89,7 @@ extern bool extent_scan_read (struct extent_scan *scan) { unsigned int si = 0; - struct extent_info *last_ei IF_LINT ( = scan->ext_info); + struct extent_info *last_ei = scan->ext_info; while (true) { @@ -127,8 +127,14 @@ extent_scan_read (struct extent_scan *scan) assert (scan->ei_count <= SIZE_MAX - fiemap->fm_mapped_extents); scan->ei_count += fiemap->fm_mapped_extents; - scan->ext_info = xnrealloc (scan->ext_info, scan->ei_count, - sizeof (struct extent_info)); + { + /* last_ei points into a buffer that may be freed via xnrealloc. + Record its offset and adjust after allocation. */ + size_t prev_idx = last_ei - scan->ext_info; + scan->ext_info = xnrealloc (scan->ext_info, scan->ei_count, + sizeof (struct extent_info)); + last_ei = scan->ext_info + prev_idx; + } unsigned int i = 0; for (i = 0; i < fiemap->fm_mapped_extents; i++) diff --git a/tests/cp/fiemap-FMR.sh b/tests/cp/fiemap-FMR.sh new file mode 100755 index 0000000000..10c9e05710 --- /dev/null +++ b/tests/cp/fiemap-FMR.sh @@ -0,0 +1,31 @@ +#!/bin/sh +# Trigger a free-memory read bug in cp from coreutils-[8.11..8.19] + +# Copyright (C) 2012 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ cp + +require_valgrind_ +require_perl_ +: ${PERL=perl} + +$PERL -e 'for (1..600) { sysseek (*STDOUT, 4096, 1)' \ + -e '&& syswrite (*STDOUT, "a" x 1024) or die "$!"}' > j || fail=1 +valgrind --quiet --error-exitcode=3 cp j j2 || fail=1 +cmp j j2 || fail=1 + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index 486bf312b7..5a237fa714 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -418,6 +418,7 @@ all_tests = \ tests/cp/existing-perm-race.sh \ tests/cp/fail-perm.sh \ tests/cp/fiemap-empty.sh \ + tests/cp/fiemap-FMR.sh \ tests/cp/fiemap-perf.sh \ tests/cp/fiemap-2.sh \ tests/cp/file-perm-race.sh \