[Bug-cpio] Corrupted files in copy-pass mode with cpio 2.4.2 and 2.5

[Holger Fleischmann]

Hallo,

we would like to contribute a bug report and a possible bug fix concerning a
problem with GNU cpio 2.4.2 and also 2.5.


**  Description of the bug:


We use cpio in copy-pass mode (cpio -pdu ...) to create backups on linux
systems. In rare cases we have the problem that the contents of
consecutively copied files on the backup are corrupted. This starts at an
arbitrary file and goes on to the end of the cpio-run.
The kind of corruption is interresting: the content of a corrupted file is
shifted beyond file boundaries. A corrupted file begins with x bytes from
the end of the preceding file and the last x bytes of the file are cut of
and shifted to the beginning of the next copied file etc.

We found out that the problem occurs when a file grows in size while cpio is
copying this file. Then all files copied after that file in the same
cpio-run will be corrupted.

As a workaround we use cpio in copy-out mode piped to cpio in copy-in mode.
Using this method the growth of files seems to be no problem.


**  Reproducing the bug:


To reproduce the bug create an empty directory with some files X1, X2, X3
with known contents (e.g. "abcdefghijklmnopqrstuvwxyz") and a large file Y
that takes some time to copy.

Go to the directory and start the copy in a shell:
  find . | cpio -pduv /tmp/testtarget

In another shell let the file Y grow some bytes WHILE CPIO IS COPYING Y:
  echo -n "1234" >>Y

The copied files will all have the correct size, but the their contents will
be shifted by 4 bytes beginning with file X1.


**  Possible bug-fix:


We looked at the source-code of cpio 2.5 and think that the problem can be
found in function copy_files_disk_to_disk() in util.c:539:

Original code fragment from GNU cpio 2.5:

...
  original_num_bytes = num_bytes;
  while (num_bytes > 0)
    {
      if (input_size == 0)
539:    if (rc = disk_fill_input_buffer (in_des, DISK_IO_BLOCK_SIZE))
          {
            if (rc > 0)
              error (0, 0, "File %s shrunk by %ld bytes, padding with zeros",
                                filename, num_bytes);
            else
              error (0, 0, "Read error at byte %ld in file %s, padding with 
zeros",
                        original_num_bytes - num_bytes, filename);
            write_nuls_to_file (num_bytes, out_des);
            break;
          }
550:  size = (input_size < num_bytes) ? input_size : num_bytes;
      if (crc_i_flag)
        {
          for (k = 0; k < size; ++k)
            crc += in_buff[k] & 0xff;
        }
556:  disk_buffered_write (in_buff, out_des, size);
      num_bytes -= size;
      input_size -= size;
      in_buff += size;
    }
...

When coping the last block of a file that grew since the start of its copy,
line 539 reads in more than the remaining num_bytes bytes. However, in line
556 only num_bytes bytes are written out (since input_size>num_bytes in line
550 and so size==num_bytes in line 556). Hence some bytes remain in the
input buffer and will be written out at the beginning of the next copied
file. Since the input buffer is not empty when starting to copy the next
file, the last bytes of the next file will also remain in the buffer etc.

We think the bug could be fixed as follows:
...
539:    if (rc = disk_fill_input_buffer (in_des, num_bytes))
...

This would prevent reading to many bytes if the file grew meanwhile. It
should be no problem that num_bytes may be greater than DISK_IO_BLOCK_SIZE
since disk_fill_input_buffer() bounds the value to DISK_IO_BLOCK_SIZE.

Tests with this modification were successful, but we are not absolutely sure
that the fix does not introduce new problems.

Best regards,
Holger Fleischmann

--
Holger Fleischmann
MAN Roland Druckmaschinen AG