> -----Original Message----- > From: Dan Williams [mailto:dan.j.williams@xxxxxxxxx] > Sent: Wednesday, January 04, 2012 5:41 PM > To: Dorau, Lukasz > Cc: neilb@xxxxxxx; linux-raid@xxxxxxxxxxxxxxx; Labun, Marcin; Ciechanowski, Ed > Subject: Re: [PATCH] imsm: fix: correct calculation of UUID > > 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? > On all disks. > > 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? > Yes, only when adding the disk via OROM. Regards, Lukasz -- 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
