Joseph Qi
2013-May-27 11:32 UTC
[Ocfs2-devel] [patch 1/4] ocfs2: fix a couple of memory leaks at o2hb_map_slot_data()
These memory will be freed in o2hb_region_release. If we free them here, then it will lead to double freed issue.> This patch can fix a couple of potential memory leaks at > o2hb_map_slot_data(). > > Signed-off-by: Jie Liu <jeff.liu at oracle.com> > > --- > fs/ocfs2/cluster/heartbeat.c | 33 +++++++++++++++++++++++++-------- > 1 files changed, 25 insertions(+), 8 deletions(-) > > diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c > index a4e855e..1b6ce53 100644 > --- a/fs/ocfs2/cluster/heartbeat.c > +++ b/fs/ocfs2/cluster/heartbeat.c > @@ -1638,7 +1638,7 @@ static void o2hb_init_region_params(struct > o2hb_region *reg) > > static int o2hb_map_slot_data(struct o2hb_region *reg) > { > - int i, j; > + int ret = 0, i, j; > unsigned int last_slot; > unsigned int spp = reg->hr_slots_per_page; > struct page *page; > @@ -1654,8 +1654,8 @@ static int o2hb_map_slot_data(struct o2hb_region *reg) > reg->hr_slots = kcalloc(reg->hr_blocks, > sizeof(struct o2hb_disk_slot), GFP_KERNEL); > if (reg->hr_slots == NULL) { > - mlog_errno(-ENOMEM); > - return -ENOMEM; > + ret = -ENOMEM; > + goto free; > } > > for(i = 0; i < reg->hr_blocks; i++) { > @@ -1673,15 +1673,15 @@ static int o2hb_map_slot_data(struct o2hb_region > *reg) > reg->hr_slot_data = kcalloc(reg->hr_num_pages, sizeof(struct page *), > GFP_KERNEL); > if (!reg->hr_slot_data) { > - mlog_errno(-ENOMEM); > - return -ENOMEM; > + ret = -ENOMEM; > + goto free; > } > > for(i = 0; i < reg->hr_num_pages; i++) { > page = alloc_page(GFP_KERNEL); > if (!page) { > - mlog_errno(-ENOMEM); > - return -ENOMEM; > + ret = -ENOMEM; > + goto free; > } > > reg->hr_slot_data[i] = page; > @@ -1701,7 +1701,24 @@ static int o2hb_map_slot_data(struct o2hb_region > *reg) > } > } > > - return 0; > + return ret; > + > +free: > + mlog_errno(ret); > + > + kfree(reg->hr_tmp_block); > + kfree(reg->hr_slots); > + > + if (reg->hr_slot_data) { > + for (i = 0; i < reg->hr_num_pages; i++) { > + page = reg->hr_slot_data[i]; > + if (page) > + __free_page(reg->hr_slot_data[i]); > + } > + kfree(reg->hr_slot_data); > + } > + > + return ret; > } > > /* Read in all the slots available and populate the tracking >
Jeff Liu
2013-May-27 11:56 UTC
[Ocfs2-devel] [patch 1/4] ocfs2: fix a couple of memory leaks at o2hb_map_slot_data()
On 05/27/2013 07:32 PM, Joseph Qi wrote:> These memory will be freed in o2hb_region_release. If we free them here, > then it will lead to double freed issue.You're right. Thanks for letting me know about this. -Jeff> >> This patch can fix a couple of potential memory leaks at >> o2hb_map_slot_data(). >> >> Signed-off-by: Jie Liu <jeff.liu at oracle.com> >> >> --- >> fs/ocfs2/cluster/heartbeat.c | 33 +++++++++++++++++++++++++-------- >> 1 files changed, 25 insertions(+), 8 deletions(-) >> >> diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c >> index a4e855e..1b6ce53 100644 >> --- a/fs/ocfs2/cluster/heartbeat.c >> +++ b/fs/ocfs2/cluster/heartbeat.c >> @@ -1638,7 +1638,7 @@ static void o2hb_init_region_params(struct >> o2hb_region *reg) >> >> static int o2hb_map_slot_data(struct o2hb_region *reg) >> { >> - int i, j; >> + int ret = 0, i, j; >> unsigned int last_slot; >> unsigned int spp = reg->hr_slots_per_page; >> struct page *page; >> @@ -1654,8 +1654,8 @@ static int o2hb_map_slot_data(struct o2hb_region *reg) >> reg->hr_slots = kcalloc(reg->hr_blocks, >> sizeof(struct o2hb_disk_slot), GFP_KERNEL); >> if (reg->hr_slots == NULL) { >> - mlog_errno(-ENOMEM); >> - return -ENOMEM; >> + ret = -ENOMEM; >> + goto free; >> } >> >> for(i = 0; i < reg->hr_blocks; i++) { >> @@ -1673,15 +1673,15 @@ static int o2hb_map_slot_data(struct o2hb_region >> *reg) >> reg->hr_slot_data = kcalloc(reg->hr_num_pages, sizeof(struct page *), >> GFP_KERNEL); >> if (!reg->hr_slot_data) { >> - mlog_errno(-ENOMEM); >> - return -ENOMEM; >> + ret = -ENOMEM; >> + goto free; >> } >> >> for(i = 0; i < reg->hr_num_pages; i++) { >> page = alloc_page(GFP_KERNEL); >> if (!page) { >> - mlog_errno(-ENOMEM); >> - return -ENOMEM; >> + ret = -ENOMEM; >> + goto free; >> } >> >> reg->hr_slot_data[i] = page; >> @@ -1701,7 +1701,24 @@ static int o2hb_map_slot_data(struct o2hb_region >> *reg) >> } >> } >> >> - return 0; >> + return ret; >> + >> +free: >> + mlog_errno(ret); >> + >> + kfree(reg->hr_tmp_block); >> + kfree(reg->hr_slots); >> + >> + if (reg->hr_slot_data) { >> + for (i = 0; i < reg->hr_num_pages; i++) { >> + page = reg->hr_slot_data[i]; >> + if (page) >> + __free_page(reg->hr_slot_data[i]); >> + } >> + kfree(reg->hr_slot_data); >> + } >> + >> + return ret; >> } >> >> /* Read in all the slots available and populate the tracking >> >