2012/1/4 Dorau, Lukasz <lukasz.dorau@xxxxxxxxx>:
> On Wed, Jan 4, 2012 at 11:05 AM, Dan Williams <dan.j.williams@xxxxxxxxx> wrote:
>>
>> On Wed, Jan 4, 2012 at 1:40 AM, Lukasz Dorau <lukasz.dorau@xxxxxxxxx>
>> wrote:
>> > The UUID is calculated from two numbers and two strings stored in arrays.
>> > Whole arrays of chars are taken to calculation (not only the used part).
>> > This is wrong, because the unused part of arrays can be changed accidentally.
>> > For example it can sometimes happen when degraded raid is being rebuilt in
>> OROM.
>> > Such accidental change of UUID causes that system installed on raid
>> > does not boot due to not recognized booting device (raid with changed UUID).
>> >
>> > Signed-off-by: Lukasz Dorau <lukasz.dorau@xxxxxxxxx>
>> > ---
>> > super-intel.c | 7 +++++--
>> > 1 files changed, 5 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/super-intel.c b/super-intel.c
>> > index 0e9269f..7f34542 100644
>> > --- a/super-intel.c
>> > +++ b/super-intel.c
>> > @@ -1820,6 +1820,7 @@ static void uuid_from_super_imsm(struct supertype
>> *st, int uuid[4])
>> > struct sha1_ctx ctx;
>> > struct imsm_dev *dev = NULL;
>> > __u32 family_num;
>> > + size_t len;
>> >
>> > /* some mdadm versions failed to set ->orig_family_num, in which
>> > * case fall back to ->family_num. orig_family_num will be
>> > @@ -1829,14 +1830,16 @@ static void uuid_from_super_imsm(struct
>> supertype *st, int uuid[4])
>> > if (family_num == 0)
>> > family_num = super->anchor->family_num;
>> > sha1_init_ctx(&ctx);
>> > - sha1_process_bytes(super->anchor->sig, MPB_SIG_LEN, &ctx);
>> > + len = strnlen((const char *)super->anchor->sig, MPB_SIG_LEN);
>> > + sha1_process_bytes(super->anchor->sig, len, &ctx);
>> > sha1_process_bytes(&family_num, sizeof(__u32), &ctx);
>> > if (super->current_vol >= 0)
>> > dev = get_imsm_dev(super, super->current_vol);
>> > if (dev) {
>> > __u32 vol = super->current_vol;
>> > sha1_process_bytes(&vol, sizeof(vol), &ctx);
>> > - sha1_process_bytes(dev->volume, MAX_RAID_SERIAL_LEN, &ctx);
>> > + len = strnlen((const char *)dev->volume, MAX_RAID_SERIAL_LEN);
>> > + sha1_process_bytes(dev->volume, len, &ctx);
>>
>> Can't do this. It will break the UUIDs of existing arrays in the field.
>>
> So we can't fill the unused part of these arrays with '\000' character
> before calculating the UUID either, can we?
>
>> When can the unused portion of anchor->sig and dev->volume be "changed
>> accidentally", as you mention, during a rebuild?
>>
> Please consider the following scenario:
> 1. Create RAID 10 volume of name "r10d4n0s64" in OROM.
> 2. Delete the volume "r10d4n0s64" in OROM.
> 3. Create RAID 10 volume of name "raid10" in OROM.
> 4. Boot the system.
> 5. Run mdadm, contents of dev->volume array is "raid10\000s64\000\000..." (8th, 9th, 10th bytes are "s64")
> 6. Fail one of the "raid10" volume's disks. The "raid10" volume is degraded now.
> 7. Restart the computer and enter OROM.
> 8. Add a new empty disk to the volume in OROM. The status of the volume is changed to "Rebuild".
> 9. Boot the system.
> 10. Run mdadm, contents of dev->volume array is "raid10\000\000\000\000\000..." (8th, 9th, 10th bytes are "\000\000\000").
On all disks or just the newly added spare?
> Now the UUID is different than it was in point 5th (above). If the operating system were installed on that volume, it would not boot due to not recognized booting device (raid with changed UUID).
...and you can only reproduce this when adding the disk via orom?
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
