resurrecting-open-source-projects / dcfldd

Enhanced version of dd for forensics and security
GNU General Public License v2.0
90 stars 19 forks source link

dcfldd: Fails to get size of destination correctly, hilarity ensues #6

Closed eribertomota closed 4 years ago

eribertomota commented 4 years ago

Bug from Debian[1].

[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=886647

The original bug says:

Here is what I get when running dcfldd from one 50GB partition to another:

[0% of 28213188Mb] 400 blocks (400Mb) written. 117:33:11 remaining.

/proc/partitions has this for the partition in question: 8 2 56426376 sda2 i.e. it's 56 GiB and not 28TiB as dcfldd seems to think. It doesn't actually take 117 hours to complete just the usual 30 minutes.

The command line is: dcfldd bs=1M if=/dev/sda2 of=/dev/sdf2 sizeprobe=if statusinterval=100

so I'm guessing that the sizeprobe is the problem.

I was able to reproduce it using the following command:

# dcfldd bs=1M if=/dev/sdb3 of=/home/test sizeprobe=if statusinterval=100
[0% of 14747648Mb] 300 blocks (300Mb) written. 68:16:29 remaining.

The /dev/sda3 has 28.1 GB, not 14.4 TB.

The next thread in the bug says:

Hi, The cause of this bug is on
debian/patches/10_fix-probing-of-large-block-devices.patch so it is
Debian specific. The patch changes from using BLKGETSIZE to
BLKGETSIZE64 without correcting for the fact that BLKGETSIZE returns
the 512 byte sector size and BLKGETSIZE64 returns the byte size. This
is from /usr/include/linux/fs.h:

#define BLKGETSIZE _IO(0x12,96) /* return device size /512 (long *arg) */
#define BLKGETSIZE64 _IOR(0x12,114,size_t)      /* return device size  
in bytes (u64 *arg) */

and this is the patch:

--- dcfldd-1.3.4.1.orig/sizeprobe.c
+++ dcfldd-1.3.4.1/sizeprobe.c
@@ -63,9 +63,13 @@ static off_t midpoint(off_t a, off_t b,
 static off_t get_dev_size(int fd, long blksize)
 {
     off_t num_sectors = 0;
-
-    if (ioctl(fd, BLKGETSIZE, &num_sectors))
-        log_info("%s: ioctl call to BLKGETSIZE failed.\n", program_name);
+
+    /*
+     * Use BLKGETSIZE64 unconditionally, since dcfldd.h #defines  
_FILE_OFFSET_BITS 64
+     * and off_t is guaranteed to be large enough to hold the result.
+     */
+    if (ioctl(fd, BLKGETSIZE64, &num_sectors))
+        log_info("%s: ioctl call to BLKGETSIZE64 failed.\n", program_name);
     else
         return (num_sectors * 512);
 }

Should be a trivial fix

However, this patch is not compatible with dcfldd now.

Eriberto

davidpolverari commented 4 years ago

This was a problem with the Debian patch (10_fix-probing-of-large-block-devices.patch). That patch is unnecessary on the current dcfldd github version (https://github.com/resurrecting-open-source-projects/dcfldd/commit/cf07652dadff37e05909d0c83f82a3ab874a48c8 at this point in time), as it has been included on the project through the commit 0f4bb57), and that's the reason why it can't be reapplied on dcfldd.

Anyways, I have a PR in a branch I'm going to push to the repo that I tested locally and it seems to have fixed it.

davidpolverari commented 4 years ago

I tested it here and it seemed to work ok now. Please, give it a try and see if it's ok there. Once everything is fine, I'll merge it onto master.

eribertomota commented 4 years ago

Hi @davidpolverari,

I confirm that these changes solve the issue.

Thanks for this fix.

davidpolverari commented 4 years ago

Hi! You're welcome. Already merged the fix onto master.