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

Previous Next

Full log

View this message in rfc822 format

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Dmitry Monakhov <dmonakhov <at> openvz.org>
Subject: bug#21790: closed (Re: bug#21790: [PATCH] coreutils/cp: handle
 EOF extents correctly)
Date: Fri, 20 Nov 2015 18:28:02 +0000

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

Your bug report

#21790: [PATCH] coreutils/cp: handle EOF extents correctly

which was filed against the coreutils package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to 21790 <at> debbugs.gnu.org.

-- 
21790: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=21790
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems

[Message part 2 (message/rfc822, inline)]

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 3 (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)]

[Message part 5 (message/rfc822, inline)]

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

Previous Next

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