Re: [PATCH 4/5] md: Turn rdev->sb_offset into a sector-based quantity.

[Andre Noll <maan@xxxxxxxxxxxxxxx>]

On 21:15, Neil Brown wrote:
> >  	if (minor_version
> > -	    && rdev->data_offset < sb_offset + (rdev->sb_size/512))
> > +	    && rdev->data_offset < sb_start / 2 + (rdev->sb_size/512))
> >  		return -EINVAL;
> 
> That doesn't look right.
>   rdev->data_offset is in sectors.
>   rdev->sb_size/512 is in sectors.
>   sb_offset is in K.
> We had a bug there.  Better fix it by dropping the "/ 2".
> 
> > @@ -3431,16 +3428,16 @@ static int do_md_run(mddev_t * mddev)
> >  		 * We don't want the data to overlap the metadata,
> >  		 * Internal Bitmap issues has handled elsewhere.
> >  		 */
> > -		if (rdev->data_offset < rdev->sb_offset) {
> > +		if (rdev->data_offset < rdev->sb_start / 2) {
> 
> And another bug.  We don't want the "/ 2".  data_offset was always
> sectors, and now sb_offset is too.

It might be worth to get fixes for these two bugs into -stable.

Here's a patch against mainline. Please forward to stable@... if
you agree.

Andre

commit c1be9a2d52dd2db1cf284e62d5e1cb8423482fb4
Author: Andre Noll <maan@xxxxxxxxxxxxxxx>
Date:   Thu Jul 17 11:33:40 2008 +0200

    md: Fix two bugs in md.c
    
    sb_offset is kilobytes while data_offset and sb_size/512 are in
    sectors.
    
    Spotted by Neil while reviewing the sector_t conversion patches.

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 2580ac1..0c7aeb9 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1113,7 +1113,7 @@ static int super_1_load(mdk_rdev_t *rdev, mdk_rdev_t *refdev, int minor_version)
 		rdev->sb_size = (rdev->sb_size | bmask) + 1;
 
 	if (minor_version
-	    && rdev->data_offset < sb_offset + (rdev->sb_size/512))
+	    && rdev->data_offset < sb_offset * 2 + (rdev->sb_size/512))
 		return -EINVAL;
 
 	if (sb->level == cpu_to_le32(LEVEL_MULTIPATH))
@@ -3432,7 +3432,7 @@ static int do_md_run(mddev_t * mddev)
 		 * We don't want the data to overlap the metadata,
 		 * Internal Bitmap issues has handled elsewhere.
 		 */
-		if (rdev->data_offset < rdev->sb_offset) {
+		if (rdev->data_offset < rdev->sb_offset * 2) {
 			if (mddev->size &&
 			    rdev->data_offset + mddev->size*2
 			    > rdev->sb_offset*2) {
-- 
The only person who always got his work done by Friday was Robinson Crusoe

Attachment: signature.asc
Description: Digital signature