From eea6b49210edf69682b2d0606bee17bbccb3765b Mon Sep 17 00:00:00 2001 From: Dmitry Monakhov Date: Fri, 30 Oct 2015 22:04:46 +0000 Subject: [PATCH] copy: fix copying of extents beyond the apparent file size fallocate can allocate extents beyond EOF via FALLOC_FL_KEEP_SIZE. Where there is a gap (hole) between the extents, and EOF is within that gap, the final hole wasn't reproduced, resulting in silent data corruption in the copied file (size too small). * src/copy.c (extent_copy): Ensure we don't process extents beyond the apparent file size, since processing and allocating those is not currently supported. * tests/cp/fiemap-extents.sh: Renamed from tests/cp/fiemap-empty.sh and re-enable parts checking the extents at and beyond EOF. * tests/local.mk: Reference the renamed test. Fixes http://bugs.gnu.org/21790 --- src/copy.c | 19 ++++++++- tests/cp/fiemap-empty.sh | 102 -------------------------------------------- tests/cp/fiemap-extents.sh | 81 +++++++++++++++++++++++++++++++++++ tests/local.mk | 2 +- 5 files changed, 100 insertions(+), 104 deletions(-) delete mode 100755 tests/cp/fiemap-empty.sh create mode 100755 tests/cp/fiemap-extents.sh diff --git a/src/copy.c b/src/copy.c index dc1cd29..6771bb5 100644 --- a/src/copy.c +++ b/src/copy.c @@ -432,6 +432,20 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t buf_size, ext_len = 0; } + /* Truncate extent to EOF. Extents starting after EOF are + treated as zero length extents starting right after EOF. + Generally this will trigger with an extent starting after + src_total_size, and result in creating a hole or zeros until EOF. + Though in a file in which extents have changed since src_total_size + was determined, we might have an extent spanning that size, + in which case we'll only copy data up to that size. */ + if (src_total_size < ext_start + ext_len) + { + if (src_total_size < ext_start) + ext_start = src_total_size; + ext_len = src_total_size - ext_start; + } + hole_size = ext_start - last_ext_start - last_ext_len; wrote_hole_at_eof = false; @@ -495,14 +509,17 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t buf_size, off_t n_read; empty_extent = false; last_ext_len = ext_len; + bool read_hole; if ( ! sparse_copy (src_fd, dest_fd, buf, buf_size, sparse_mode == SPARSE_ALWAYS, src_name, dst_name, ext_len, &n_read, - &wrote_hole_at_eof)) + &read_hole)) goto fail; dest_pos = ext_start + n_read; + if (n_read) + wrote_hole_at_eof = read_hole; } /* If the file ends with unwritten extents not accounted for in the deleted file mode 100755 index b3b2cd7..0000000 --- a/tests/cp/fiemap-empty.sh +++ /dev/null @@ -1,101 +0,0 @@ -#!/bin/sh -# Test cp reads unwritten extents efficiently - -# Copyright (C) 2011-2013 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 - -# FIXME: enable any part of this test that is still relevant, -# or, if none are relevant (now that cp does not handle unwritten -# extents), just remove the test altogether. -skip_ 'disabled for now' - -touch fiemap_chk -fiemap_capable_ fiemap_chk || - skip_ 'this file system lacks FIEMAP support' -rm fiemap_chk - -# TODO: rather than requiring $(fallocate), possible add -# this functionality to truncate --alloc -fallocate --help >/dev/null || skip_ 'The fallocate utility is required' -fallocate -l 1 -n falloc.test || - skip_ 'this file system lacks FALLOCATE support' -rm falloc.test - -# Require more space than we'll actually use, so that -# tests run in parallel do not run out of space. -# Otherwise, with inadequate space, simply running the following -# fallocate command would induce a temporary disk-full condition, -# which would cause failure of unrelated tests run in parallel. -require_file_system_bytes_free_ 800000000 - -fallocate -l 600MiB space.test || - skip_ 'this test needs at least 600MiB free space' - -# Disable this test on old BTRFS (e.g. Fedora 14) -# which reports ordinary extents for unwritten ones. -filefrag space.test || skip_ 'the 'filefrag' utility is missing' -filefrag -v space.test | grep -F 'unwritten' > /dev/null || - skip_ 'this file system does not report empty extents as "unwritten"' - -rm space.test - -# Ensure we read a large empty file quickly -fallocate -l 300MiB empty.big || framework_failure_ -timeout 3 cp --sparse=always empty.big cp.test || fail=1 -test $(stat -c %s empty.big) = $(stat -c %s cp.test) || fail=1 -rm empty.big cp.test - -# Ensure we handle extents beyond file size correctly. -# Note until we support fallocate, we will not maintain -# the file allocation. FIXME: amend this test when fallocate is supported. -fallocate -l 10MiB -n unwritten.withdata || framework_failure_ -dd count=10 if=/dev/urandom conv=notrunc iflag=fullblock of=unwritten.withdata -cp unwritten.withdata cp.test || fail=1 -test $(stat -c %s unwritten.withdata) = $(stat -c %s cp.test) || fail=1 -cmp unwritten.withdata cp.test || fail=1 -rm unwritten.withdata cp.test - -# The following to generate unaccounted extents followed by a hole, is not -# supported by ext4 at least. The ftruncate discards all extents not -# accounted for in the size. -# fallocate -l 10MiB -n unacc.withholes -# dd count=10 if=/dev/urandom conv=notrunc iflag=fullblock of=unacc.withholes -# truncate -s20M unacc.withholes - -# Ensure we handle a hole after empty extents correctly. -# Since all extents are accounted for in the size, -# we can maintain the allocation independently from -# fallocate() support. -fallocate -l 10MiB empty.withholes -truncate -s 20M empty.withholes -sectors_per_block=$(expr $(stat -c %o .) / 512) -cp empty.withholes cp.test || fail=1 -test $(stat -c %s empty.withholes) = $(stat -c %s cp.test) || fail=1 -# These are usually equal but can vary by an IO block due to alignment -alloc_diff=$(expr $(stat -c %b empty.withholes) - $(stat -c %b cp.test)) -alloc_diff=$(echo $alloc_diff | tr -d -- -) # abs() -test $alloc_diff -le $sectors_per_block || fail=1 -# Again with SPARSE_ALWAYS -cp --sparse=always empty.withholes cp.test || fail=1 -test $(stat -c %s empty.withholes) = $(stat -c %s cp.test) || fail=1 -# cp.test should take 0 space, but allowing for some systems -# that store default extended attributes in data blocks -test $(stat -c %b cp.test) -le $sectors_per_block || fail=1 -rm empty.withholes cp.test - -Exit $fail diff --git a/tests/cp/fiemap-extents.sh b/tests/cp/fiemap-extents.sh new file mode 100755 index 0000000..55ec5df --- /dev/null +++ b/tests/cp/fiemap-extents.sh @@ -0,0 +1,81 @@ +#!/bin/sh +# Test cp handles extents correctly + +# Copyright (C) 2011-2015 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_sparse_support_ + +touch fiemap_chk +fiemap_capable_ fiemap_chk || + skip_ 'this file system lacks FIEMAP support' +rm fiemap_chk + +fallocate --help >/dev/null || skip_ 'The fallocate utility is required' +touch falloc.test || framework_failure_ +fallocate -l 1 -o 0 -n falloc.test || + skip_ 'this file system lacks FALLOCATE support' +rm falloc.test + +# We don't currently handle unwritten extents specially +if false; then +# Require more space than we'll actually use, so that +# tests run in parallel do not run out of space. +# Otherwise, with inadequate space, simply running the following +# fallocate command would induce a temporary disk-full condition, +# which would cause failure of unrelated tests run in parallel. +require_file_system_bytes_free_ 800000000 + +fallocate -l 600MiB space.test || + skip_ 'this test needs at least 600MiB free space' + +# Disable this test on old BTRFS (e.g. Fedora 14) +# which reports ordinary extents for unwritten ones. +filefrag space.test || skip_ 'the 'filefrag' utility is missing' +filefrag -v space.test | grep -F 'unwritten' > /dev/null || + skip_ 'this file system does not report empty extents as "unwritten"' + +rm space.test + +# Ensure we read a large empty file quickly +fallocate -l 300MiB empty.big || framework_failure_ +timeout 3 cp --sparse=always empty.big cp.test || fail=1 +test $(stat -c %s empty.big) = $(stat -c %s cp.test) || fail=1 +rm empty.big cp.test +fi + +# Ensure we handle extents beyond file size correctly. +# Note until we support fallocate, we will not maintain +# the file allocation. FIXME: amend this test if fallocate is supported. +# Note currently this only uses fiemap logic when the allocation (-l) +# is smaller than the size, thus identifying the file as sparse. +# Note the '-l 1' case is an effective noop, and just checks +# a file with a trailing hole is copied correctly. +for sparse_mode in always auto never; do + for alloc in '-l 4MiB ' '-l 1MiB -o 4MiB' '-l 1'; do + dd count=10 if=/dev/urandom iflag=fullblock of=unwritten.withdata + truncate -s 2MiB unwritten.withdata || framework_failure_ + fallocate $alloc -n unwritten.withdata || framework_failure_ + cp --sparse=$sparse_mode unwritten.withdata cp.test || fail=1 + test $(stat -c %s unwritten.withdata) = $(stat -c %s cp.test) || fail=1 + cmp unwritten.withdata cp.test || fail=1 + rm unwritten.withdata cp.test + done +done + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index adf96f0..89fdbb0 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -446,7 +446,7 @@ all_tests = \ tests/cp/existing-perm-dir.sh \ tests/cp/existing-perm-race.sh \ tests/cp/fail-perm.sh \ - tests/cp/fiemap-empty.sh \ + tests/cp/fiemap-extents.sh \ tests/cp/fiemap-FMR.sh \ tests/cp/fiemap-perf.sh \ tests/cp/fiemap-2.sh \ -- 1.7.2.5