--- Begin Message ---
|
Subject: |
[PATCH] coreutils/cp: handle EOF extents correctly |
|
Date: |
Fri, 30 Oct 2015 13:02:39 +0400 |
fallocate can allocate extens beyond EOF via FALLOC_FL_KEEP_SIZE.
Currenly sparse engine tries to copy such extents which is wrong and
result in silent data corruption (leave file with incorrect size).
##TESTCASE
echo blabla > sparse_falloc.in
truncate -s 2M sparse_falloc.in
fallocate -n -o 4M -l 1M sparse_falloc.in
cp sparse_falloc.in sparse_falloc.out
cmp sparse_falloc.in sparse_falloc.out
Signed-off-by: Dmitry Monakhov <address@hidden>
---
src/copy.c | 7 ++++++-
tests/cp/sparse-unwritten.sh | 29 +++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletion(-)
create mode 100755 tests/cp/sparse-unwritten.sh
diff --git a/src/copy.c b/src/copy.c
index edf022e..de197a4 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -430,7 +430,12 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t
buf_size,
ext_start = last_ext_start + scan.ext_info[i].ext_length;
ext_len = 0;
}
-
+ /* Is this extent beyond EOF? */
+ if (ext_start + ext_len >= src_total_size)
+ {
+ wrote_hole_at_eof = true;
+ break;
+ }
ext_hole_size = ext_start - last_ext_start - last_ext_len;
wrote_hole_at_eof = false;
diff --git a/tests/cp/sparse-unwritten.sh b/tests/cp/sparse-unwritten.sh
new file mode 100755
index 0000000..4666535
--- /dev/null
+++ b/tests/cp/sparse-unwritten.sh
@@ -0,0 +1,29 @@
+#!/bin/sh
+# Test cp for a file which has extents beyond EOF
+
+# Copyright (C) 2006-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 <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ cp
+require_sparse_support_
+
+echo blabla > sparse_falloc.in || framework_failure_
+truncate -s 2M sparse_falloc.in || framework_failure_
+fallocate -n -o 4M -l 1M sparse_falloc.in || framework_failure_
+cp sparse_falloc.in sparse_falloc.out || fail=1
+cmp sparse_falloc.in sparse_falloc.out || fail=1
+
+Exit $fail
--
2.1.4
--- End Message ---
--- Begin Message ---
|
Subject: |
Re: bug#21790: [PATCH] coreutils/cp: handle EOF extents correctly |
|
Date: |
Fri, 20 Nov 2015 18:26:57 +0000 |
|
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
On 30/10/15 18:54, Pádraig Brady wrote:
> On 30/10/15 16:57, Pádraig Brady wrote:
>> On 30/10/15 09:02, Dmitry Monakhov wrote:
>>> fallocate can allocate extens beyond EOF via FALLOC_FL_KEEP_SIZE.
>>> Currenly sparse engine tries to copy such extents which is wrong and
>>> result in silent data corruption (leave file with incorrect size).
>>>
>>> ##TESTCASE
>>> echo blabla > sparse_falloc.in
>>> truncate -s 2M sparse_falloc.in
>>> fallocate -n -o 4M -l 1M sparse_falloc.in
>>> cp sparse_falloc.in sparse_falloc.out
>>> cmp sparse_falloc.in sparse_falloc.out
>>
>> Ouch. Thanks for the analysis and patch.
>> It looks correct. I'll analyze further before applying.
>
> This doesn't handle the --sparse==never case
> (which is broken with and without the patch).
>
> Also one might have an extent spanning the file size boundary,
> in which this patch could miss the remaining data?
>
> Also currently if the source file is being extended
> while being copied, we continue to read, whereas we now wont.
> Theoretically this is an issue when st_size doesn't match
> what's available to be read, though maybe not a practical issue
> since we don't use this path for a zero st_size, nor
> sources that don't support fiemap anyway.
>
> This might be an opportune time to rip out the fiemap stuff
> in favor of SEEK{DATA,HOLE} anyway, which I intended to do
> during this cycle. For fix backporting sake though,
> we should apply the minimal fix to the fiemap code first.
>
> Attached is the current minimally tested patch.
I'll apply the attached in your name soon.
Please review.
thanks,
Pádraig.
cp-sparse-extents.patch
Description: Text Data
--- End Message ---