RE: [PATCH] imsm: FIX: crash during getting map

["Kwolek, Adam" <adam.kwolek@intel.com>]

Could you give me branch name were the changes can be visible?

Thanks
Adam


> -----Original Message-----
> From: linux-raid-owner@vger.kernel.org [mailto:linux-raid-
> owner@vger.kernel.org] On Behalf Of NeilBrown
> Sent: Thursday, February 03, 2011 7:04 AM
> To: Kwolek, Adam
> Cc: linux-raid@vger.kernel.org; Williams, Dan J; Ciechanowski, Ed;
> Neubauer, Wojciech
> Subject: Re: [PATCH] imsm: FIX: crash during getting map
> 
> On Tue, 01 Feb 2011 08:57:37 +0100 Adam Kwolek <adam.kwolek@intel.com>
> wrote:
> 
> > When get_imsm_map() is called with second_map parameter == '-1'
> > and array is not in migration state NULL pointer is returned.
> > This is wrong. '-1' means return map as migration record points.
> >
> > '-1' can be passed to get_imsm_map() from imsm_num_data_members().
> > imsm_num_data_members() is called to get current map members based
> > on migr_state information
> >
> > Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
> > ---
> >
> >  super-intel.c |    7 ++++---
> >  1 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/super-intel.c b/super-intel.c
> > index 84ab47b..ee0d9c4 100644
> > --- a/super-intel.c
> > +++ b/super-intel.c
> > @@ -567,15 +567,16 @@ struct imsm_map *get_imsm_map(struct imsm_dev
> *dev, int second_map)
> >  {
> >  	struct imsm_map *map = &dev->vol.map[0];
> >
> > -	if (second_map && !dev->vol.migr_state)
> > +	if ((second_map == 1) && !dev->vol.migr_state)
> >  		return NULL;
> > -	else if (second_map) {
> > +	else if ((second_map == 1) ||
> > +		 ((second_map < 0) && (dev->vol.migr_state))) {
> >  		void *ptr = map;
> >
> >  		return ptr + sizeof_imsm_map(map);
> >  	} else
> >  		return map;
> > -
> > +
> >  }
> >
> >  /* return the size of the device.
> 
> Thanks.
> I added some comments and took the opportunity to simplify
> get_imsm_ord_tbl_ent.  See below.
> 
> NeilBrown
> 
> commit 5e7b0330669594ee79201d19ff45a7850fa0f951
> Author: Adam Kwolek <adam.kwolek@intel.com>
> Date:   Thu Feb 3 17:02:39 2011 +1100
> 
>     imsm: FIX: crash during getting map
> 
>     When get_imsm_map() is called with second_map parameter == '-1'
>     and array is not in migration state NULL pointer is returned.
>     This is wrong. '-1' means return map as migration record points.
> 
>     '-1' can be passed to get_imsm_map() from imsm_num_data_members().
>     imsm_num_data_members() is called to get current map members based
>     on migr_state information
> 
>     Signed-off-by: Adam Kwolek <adam.kwolek@intel.com>
>     Signed-off-by: NeilBrown <neilb@suse.de>
> 
> diff --git a/super-intel.c b/super-intel.c
> index 84ab47b..4081071 100644
> --- a/super-intel.c
> +++ b/super-intel.c
> @@ -565,17 +565,24 @@ static size_t sizeof_imsm_map(struct imsm_map
> *map)
> 
>  struct imsm_map *get_imsm_map(struct imsm_dev *dev, int second_map)
>  {
> +	/* A device can have 2 maps if it is in the middle of a
> migration.
> +	 * If second_map is:
> +	 *    0   - we return the first map
> +	 *    1   - we return the second map if it exists, else NULL
> +	 *   -1   - we return the second map if it exists, else the first
> +	 */
>  	struct imsm_map *map = &dev->vol.map[0];
> 
> -	if (second_map && !dev->vol.migr_state)
> +	if (second_map == 1 && !dev->vol.migr_state)
>  		return NULL;
> -	else if (second_map) {
> +	else if (second_map == 1 ||
> +		 (second_map < 0 && dev->vol.migr_state)) {
>  		void *ptr = map;
> 
>  		return ptr + sizeof_imsm_map(map);
>  	} else
>  		return map;
> -
> +
>  }
> 
>  /* return the size of the device.
> @@ -654,14 +661,7 @@ static __u32 get_imsm_ord_tbl_ent(struct imsm_dev
> *dev,
>  {
>  	struct imsm_map *map;
> 
> -	if (second_map == -1) {
> -		if (dev->vol.migr_state)
> -			map = get_imsm_map(dev, 1);
> -		else
> -			map = get_imsm_map(dev, 0);
> -	} else {
> -		map = get_imsm_map(dev, second_map);
> -	}
> +	map = get_imsm_map(dev, second_map);
> 
>  	/* top byte identifies disk under rebuild */
>  	return __le32_to_cpu(map->disk_ord_tbl[slot]);
> --
> To unsubscribe from this list: send the line "unsubscribe linux-raid"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html