* [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@vger.kernel.org, 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox