Skip to content

Commit

Permalink
Fixes calculation of largest partition byte size
Browse files Browse the repository at this point in the history
Ensures the byte offset of the final partition being backed up is correctly
calculated instead of the current situation where the variable contains a
memory address. The calculation fix prevents a sanity check from being
incorrectly triggered during a comparison operation with the disk length
approximately 32MB or less (depends on exact memory addresses)

Also fixes the sfdisk fields parsing (the 'Id' field has been renamed 'type')
and ensures the byte offset to the end partition is correctly used, so that the
.size file contains the offset to the end of the final partition. This ensures
the ability to restore a subset of partitions to a smaller drive, as was
intended by [1] but never realized in Rescuezilla until now.

Further background: The largest partition byte size calculation feature was
added during the development of v1.0.5 (by cherry-picking a git commit authored
in 2012 a separate Redo repository in 2012 [1]). During testing at the time, an
'Experimental values on scalar is now forbidden' would pop up, but without
sufficient Perl experience at the time, it was unclear that root cause was the
values() function no longer being able to operate on hash references, so this
issue was "fixed" by removing the values() function altogether, not realizing
this would mean the $ptab_bytes variable ends up with a hash reference instead
of the largest partition size.

This incorrect $ptab_bytes value gets compared to the length of the disk (from
blockdev), and the application has a sanity check which exits if it's smaller
than the disk length.

There has been no reports of application exiting due to this issue (though it
may have happened to some people). Because Rescuezilla v1.0.5/v1.0.5.1 use
32-bit memory addresses, the typical length required to trigger this issue is
~32MB or less, but it could potentially trigger on larger disks if the memory
address happened to be larger.

The proper fix is to dereference the hashref before running the values()
function as intended. Task rescuezilla#55 contains more information.

[1] 292e1d6

[2] c00448e

[3] https://perldoc.perl.org/functions/values.html

Fixes rescuezilla#55
  • Loading branch information
shasheene committed Apr 25, 2020
1 parent 903aa37 commit 60a8575
Showing 1 changed file with 8 additions and 5 deletions.
13 changes: 8 additions & 5 deletions src/livecd/chroot/usr/local/sbin/rescuezilla
Expand Up @@ -139,7 +139,7 @@ func parttab($text) {
my @text = split("\n", $text);
my %parts;
foreach my $line (@text) {
my @match = ($line =~ /^[[:blank:]]*(\/dev\/[[:alnum:]]+|-)[[:blank:]]+:[[:blank:]]*start=[[:blank:]]*([[:digit:]]+), size=[[:blank:]]*([[:digit:]]+), Id=[[:blank:]]*([[:xdigit:]]+)/);
my @match = ($line =~ /^[[:blank:]]*(\/dev\/[[:alnum:]]+|-)[[:blank:]]+:[[:blank:]]*start=[[:blank:]]*([[:digit:]]+), size=[[:blank:]]*([[:digit:]]+), type=[[:blank:]]*([[:xdigit:]]+)/);
if (@match && $match[3]) {
if (((length $match[0]) < 3) || !$match[2]) {
print "Error on line: " . $line;
Expand Down Expand Up @@ -400,13 +400,15 @@ sub do_backup {
my $bytes = `$blockdev_cmd`;
print("Unprocessed blockdev output: $bytes\n");
$bytes =~ s/\D//g;
# Get the byte offset of the end of the last partition selected
my $ptab_bytes = max(values(%{partends($ptab)}));
print "partition table reports drive size $ptab_bytes\n";
die "blockdev reports smaller drive size $bytes than determined from partition table $ptab_bytes\n" unless $bytes >= $ptab_bytes;
$bytes = $ptab_bytes;
my $quoted_filesize_path = shell_quote("$dest/$file.size");
my $save_filesize_cmd = "echo $bytes > $quoted_filesize_path";
print("Running $save_filesize_cmd\n");
system($save_filesize_cmd);
my $ptab_bytes = max(partends($ptab));
die "blockdev reports smaller drive size $bytes than determined from partition table $ptab_bytes\n" unless $bytes >= $ptab_bytes;
$bytes = $ptab_bytes;
print("Running $save_filesize_cmd\n");
system($save_filesize_cmd);
print "\t* ".loc("Size of %1 (%2 bytes) saved to %3",$src_drive,$bytes,$quoted_filesize_path)."\n";
Expand Down Expand Up @@ -854,7 +856,8 @@ sub do_restore {
} elsif ('sectors' eq fdisk_units($src_fdisk_text)) {
# If sfdisk file is present, check whether the partition can be successfully restored.
print "*** Reading original drive partition table.\n";
my $src_bytes_fdisk = max(partends($src_fdisk_text));
# Get the byte offset of the end of the last partition listed in the backup
my $src_bytes_fdisk = max(values(%{partends($src_fdisk_text)}));
if ($src_bytes_fdisk > $src_bytes) {
print "Warning: stored size of original drive $src_bytes is smaller than size calculated from original partition table $src_bytes_fdisk !\n";
}
Expand Down

0 comments on commit 60a8575

Please sign in to comment.