| Message ID | 1410084652-23031-1-git-send-email-roy.qing.li@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Sep 7, 2014 at 2:10 PM, <roy.qing.li@gmail.com> wrote: > From: Li RongQing <roy.qing.li@gmail.com> > > If the state variable is krealloced successfully, map->osd_state will be > freed, once following two reallocation failed, and exit the function > without resetting map->osd_state, map->osd_state become a wild pointer. > > fix it by resetting them after krealloc successfully. > > Signed-off-by: Li RongQing <roy.qing.li@gmail.com> > --- > net/ceph/osdmap.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c > index c547e46..81e9c66 100644 > --- a/net/ceph/osdmap.c > +++ b/net/ceph/osdmap.c > @@ -671,15 +671,19 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max) > int i; > > state = krealloc(map->osd_state, max*sizeof(*state), GFP_NOFS); > + if (!state) > + return -ENOMEM; > + map->osd_state = state; > + > weight = krealloc(map->osd_weight, max*sizeof(*weight), GFP_NOFS); > - addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS); > - if (!state || !weight || !addr) { > - kfree(state); > - kfree(weight); > - kfree(addr); > + if (!weight) > + return -ENOMEM; > + map->osd_weight = weight; > > + addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS); > + if (!addr) > return -ENOMEM; > - } > + map->osd_addr = addr; > > for (i = map->max_osd; i < max; i++) { > state[i] = 0; > @@ -687,10 +691,6 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max) > memset(addr + i, 0, sizeof(*addr)); > } > > - map->osd_state = state; > - map->osd_weight = weight; > - map->osd_addr = addr; > - > if (map->osd_primary_affinity) { > u32 *affinity; > > -- > 1.7.10.4 Looks good. I'll apply it tomorrow. Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 7, 2014 at 6:27 PM, Ilya Dryomov <ilya.dryomov@inktank.com> wrote: > On Sun, Sep 7, 2014 at 2:10 PM, <roy.qing.li@gmail.com> wrote: >> From: Li RongQing <roy.qing.li@gmail.com> >> >> If the state variable is krealloced successfully, map->osd_state will be >> freed, once following two reallocation failed, and exit the function >> without resetting map->osd_state, map->osd_state become a wild pointer. >> >> fix it by resetting them after krealloc successfully. >> >> Signed-off-by: Li RongQing <roy.qing.li@gmail.com> >> --- >> net/ceph/osdmap.c | 20 ++++++++++---------- >> 1 file changed, 10 insertions(+), 10 deletions(-) >> >> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c >> index c547e46..81e9c66 100644 >> --- a/net/ceph/osdmap.c >> +++ b/net/ceph/osdmap.c >> @@ -671,15 +671,19 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max) >> int i; >> >> state = krealloc(map->osd_state, max*sizeof(*state), GFP_NOFS); >> + if (!state) >> + return -ENOMEM; >> + map->osd_state = state; >> + >> weight = krealloc(map->osd_weight, max*sizeof(*weight), GFP_NOFS); >> - addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS); >> - if (!state || !weight || !addr) { >> - kfree(state); >> - kfree(weight); >> - kfree(addr); >> + if (!weight) >> + return -ENOMEM; >> + map->osd_weight = weight; >> >> + addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS); >> + if (!addr) >> return -ENOMEM; >> - } >> + map->osd_addr = addr; >> >> for (i = map->max_osd; i < max; i++) { >> state[i] = 0; >> @@ -687,10 +691,6 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max) >> memset(addr + i, 0, sizeof(*addr)); >> } >> >> - map->osd_state = state; >> - map->osd_weight = weight; >> - map->osd_addr = addr; >> - >> if (map->osd_primary_affinity) { >> u32 *affinity; >> >> -- >> 1.7.10.4 > > Looks good. I'll apply it tomorrow. Pushed a branch with your patch. Minor modifications: use map->* instead of local variables for initialization and change primary affinity case so it fits in. You can take a look at https://github.com/ceph/ceph-client/commit/790415a024871a2100388ce4b3d485756fb90865 Thanks, Ilya -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c index c547e46..81e9c66 100644 --- a/net/ceph/osdmap.c +++ b/net/ceph/osdmap.c @@ -671,15 +671,19 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max) int i; state = krealloc(map->osd_state, max*sizeof(*state), GFP_NOFS); + if (!state) + return -ENOMEM; + map->osd_state = state; + weight = krealloc(map->osd_weight, max*sizeof(*weight), GFP_NOFS); - addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS); - if (!state || !weight || !addr) { - kfree(state); - kfree(weight); - kfree(addr); + if (!weight) + return -ENOMEM; + map->osd_weight = weight; + addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS); + if (!addr) return -ENOMEM; - } + map->osd_addr = addr; for (i = map->max_osd; i < max; i++) { state[i] = 0; @@ -687,10 +691,6 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max) memset(addr + i, 0, sizeof(*addr)); } - map->osd_state = state; - map->osd_weight = weight; - map->osd_addr = addr; - if (map->osd_primary_affinity) { u32 *affinity;