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

[Andrei Borzenkov <arvidjaar@gmail.com>]

I committed your patch. Thank you!

11.09.2015 17:54, 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.
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel