All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] imsm: FIX: crash during getting map
@ 2011-02-01  7:57 Adam Kwolek
  2011-02-03  6:03 ` NeilBrown
  0 siblings, 1 reply; 3+ messages in thread
From: Adam Kwolek @ 2011-02-01  7:57 UTC (permalink / raw)
  To: neilb; +Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

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.


^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] imsm: FIX: crash during getting map
  2011-02-01  7:57 [PATCH] imsm: FIX: crash during getting map Adam Kwolek
@ 2011-02-03  6:03 ` NeilBrown
  2011-02-03  8:21   ` Kwolek, Adam
  0 siblings, 1 reply; 3+ messages in thread
From: NeilBrown @ 2011-02-03  6:03 UTC (permalink / raw)
  To: Adam Kwolek
  Cc: linux-raid, dan.j.williams, ed.ciechanowski, wojciech.neubauer

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]);

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* RE: [PATCH] imsm: FIX: crash during getting map
  2011-02-03  6:03 ` NeilBrown
@ 2011-02-03  8:21   ` Kwolek, Adam
  0 siblings, 0 replies; 3+ messages in thread
From: Kwolek, Adam @ 2011-02-03  8:21 UTC (permalink / raw)
  To: NeilBrown
  Cc: linux-raid, Williams, Dan J, Ciechanowski, Ed, Neubauer, Wojciech

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

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2011-02-03  8:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-02-01  7:57 [PATCH] imsm: FIX: crash during getting map Adam Kwolek
2011-02-03  6:03 ` NeilBrown
2011-02-03  8:21   ` Kwolek, Adam

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.