Re: cryptodisk: teach grub_cryptodisk_insert() about partitions (bug #45889)

[TJ]

On 11-09-2015 15:11, Andrei Borzenkov wrote:
> 09.09.2015 04:18, TJ пишет:
> > On 08-09-2015 17:38, Vladimir 'φ-coder/phcoder' Serbinenko wrote:
> > > On 06.09.2015 21:10, TJ wrote:
> > > > https://savannah.gnu.org/bugs/index.php?45889
> > > > +      if ((disk->partition && disk->partition->number ==
> > > > dev->partition_number) ||
> > > > +          (!disk->partition && dev->partition_number == 0))
> > > > +        return dev;
> > > Please store and compare partition start, not parition number as the
> > > same partition can be available several times through different 
> > > partiton
> > > schemes under different numbers. Additionally this allows to use
> > > get_partition_start which already has the logic of handling empty
> > > partitions
> >
> > Done and tested. Works perfectly.
>
> Well, should not it also compare disk sizes (grub_disk_get_size)?
> Also grub_partition_get_start cannot differentiate between full disk
> (start == 0) and partition that starts at offset 0.

My original patch differentiated based on partition_number == 0 
indicating a non-partitioned disk (assuming 1-based partition numbers).

Vladimir asked me to use grub_partition_get_start() due to multiple 
partitioning schemes. I was concerned the function has no concept of an 
error indicator but as it returns 0 when no partitions are found that is 
equivalent (although it could be argued it has dual-use if it is 
possible for a partition to start at sector 0).

In grub_cryptodisk_insert() partition_start == 0 means it is a whole 
disk.

In this if() clause the disk has already been confirmed identical and 
so the only question is whether the cryptodisk is a whole-disk or a 
partition, and if so which partition.

As the starting sector is being stored and that is a unique value 
per-disk, regardless of if there are multiple partition schemes (e.g. 
GPT + Hybrid MBR) the starting sectors will be identical.

If the partition lengths are different (in the multiple partition 
schemes) isn't that a bug in the partitioning and something grub doesn't 
need to concern itself about?

In the event of the partition matching failing the function behaves as 
it has done previously, returning the whole device.