GNU bug report logs - #21790
[PATCH] coreutils/cp: handle EOF extents correctly

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 21790 in the body.
You can then email your comments to 21790 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Monakhov <dmonakhov <at> openvz.org>
To: bug-coreutils <at> gnu.org
Cc: Dmitry Monakhov <dmonakhov <at> openvz.org>, vvs <at> parallels.com, devel <at> openvz.org
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 <dmonakhov <at> openvz.org>
---
 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

Message #8 received at 21790 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Dmitry Monakhov <dmonakhov <at> openvz.org>, 21790 <at> debbugs.gnu.org
Cc: vvs <at> parallels.com, devel <at> openvz.org
Subject: Re: bug#21790: [PATCH] coreutils/cp: handle EOF extents correctly
Date: Fri, 30 Oct 2015 16:57:04 +0000

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.

thanks!
Pádraig.

Message #11 received at 21790 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Dmitry Monakhov <dmonakhov <at> openvz.org>, 21790 <at> debbugs.gnu.org
Cc: vvs <at> parallels.com, devel <at> openvz.org
Subject: Re: bug#21790: [PATCH] coreutils/cp: handle EOF extents correctly
Date: Fri, 30 Oct 2015 18:54:53 +0000

[Message part 1 (text/plain, inline)]

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.

thanks,
Pádraig.

[cp-trailing-extents.patch (text/x-patch, attachment)]

Message #16 received at 21790-done <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Dmitry Monakhov <dmonakhov <at> openvz.org>, 21790-done <at> debbugs.gnu.org
Cc: vvs <at> parallels.com, devel <at> openvz.org
Subject: Re: bug#21790: [PATCH] coreutils/cp: handle EOF extents correctly
Date: Fri, 20 Nov 2015 18:26:57 +0000

[Message part 1 (text/plain, inline)]

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 (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.