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

From: Pádraig Brady <[email-masked]>
Date: Fri, 30 Oct 2015 18:54:53 +0000

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.
diff --git a/src/copy.c b/src/copy.c
index edf022e..f251f32 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -431,6 +431,14 @@ extent_copy (int src_fd, int dest_fd, char *buf, size_t buf_size,
               ext_len = 0;
             }
 
+          /* Truncate extent to EOF.  */
+          if (ext_start + ext_len >= src_total_size)
+            {
+              if (src_total_size < ext_start)
+                ext_start = src_total_size;
+              ext_len = src_total_size - ext_start;
+            }
+
           ext_hole_size = ext_start - last_ext_start - last_ext_len;
 
           wrote_hole_at_eof = false;
@@ -493,14 +501,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 ? hole_size: 0,
                                   true, 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