| Message ID | 20240720103349.3347764-1-yangyongpeng1@oppo.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [f2fs-dev] f2fs: free victim_secmap when pinned_secmap allocation fails | expand |
On 2024/7/20 18:33, Yongpeng Yang wrote: > In the init_victim_secmap function, if the allocation of > dirty_i->pinned_secmap fails, dirty_i->victim_secmap is not > freed, which can cause a memory leak. > > Signed-off-by: Yongpeng Yang <yangyongpeng1@oppo.com> > --- > fs/f2fs/segment.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 78c3198a6308..1e784ea3dbb4 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -4971,8 +4971,10 @@ static int init_victim_secmap(struct f2fs_sb_info *sbi) > return -ENOMEM; > > dirty_i->pinned_secmap = f2fs_kvzalloc(sbi, bitmap_size, GFP_KERNEL); > - if (!dirty_i->pinned_secmap) > + if (!dirty_i->pinned_secmap) { > + kvfree(dirty_i->victim_secmap); Yongpeng, In below path, it will release pinned_secmap/victim_secmap? - f2fs_destroy_segment_manager - destroy_victim_secmap : kvfree(dirty_i->pinned_secmap); : kvfree(dirty_i->victim_secmap); Thanks, > return -ENOMEM; > + } > > dirty_i->pinned_secmap_cnt = 0; > dirty_i->enable_pin_section = true;
On 7/22/2024 9:28 AM, Chao Yu wrote: > On 2024/7/20 18:33, Yongpeng Yang wrote: >> In the init_victim_secmap function, if the allocation of >> dirty_i->pinned_secmap fails, dirty_i->victim_secmap is not >> freed, which can cause a memory leak. >> >> Signed-off-by: Yongpeng Yang <yangyongpeng1@oppo.com> >> --- >> fs/f2fs/segment.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 78c3198a6308..1e784ea3dbb4 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -4971,8 +4971,10 @@ static int init_victim_secmap(struct >> f2fs_sb_info *sbi) >> return -ENOMEM; >> dirty_i->pinned_secmap = f2fs_kvzalloc(sbi, bitmap_size, >> GFP_KERNEL); >> - if (!dirty_i->pinned_secmap) >> + if (!dirty_i->pinned_secmap) { >> + kvfree(dirty_i->victim_secmap); > > Yongpeng, > > In below path, it will release pinned_secmap/victim_secmap? > > - f2fs_destroy_segment_manager > - destroy_victim_secmap > : kvfree(dirty_i->pinned_secmap); > : kvfree(dirty_i->victim_secmap); > > Thanks, Oh, I missed the error handler of f2fs_build_segment_manager, which will free valid pointer and ignore NULL pointer. Just get rid of this patch. > >> return -ENOMEM; >> + } >> dirty_i->pinned_secmap_cnt = 0; >> dirty_i->enable_pin_section = true;
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 78c3198a6308..1e784ea3dbb4 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -4971,8 +4971,10 @@ static int init_victim_secmap(struct f2fs_sb_info *sbi) return -ENOMEM; dirty_i->pinned_secmap = f2fs_kvzalloc(sbi, bitmap_size, GFP_KERNEL); - if (!dirty_i->pinned_secmap) + if (!dirty_i->pinned_secmap) { + kvfree(dirty_i->victim_secmap); return -ENOMEM; + } dirty_i->pinned_secmap_cnt = 0; dirty_i->enable_pin_section = true;
In the init_victim_secmap function, if the allocation of dirty_i->pinned_secmap fails, dirty_i->victim_secmap is not freed, which can cause a memory leak. Signed-off-by: Yongpeng Yang <yangyongpeng1@oppo.com> --- fs/f2fs/segment.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)