[f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager

[f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager

[Chao Yu]

A NULL point should avoid to be used in destroy_segment_manager after allocating memory fail for f2fs_sm_info.

Signed-off-by: Chao Yu <chao2.yu@samsung.com>
---
 fs/f2fs/segment.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 3d4d5fc..ff363e6
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1744,6 +1744,8 @@ static void destroy_sit_info(struct f2fs_sb_info *sbi)
 void destroy_segment_manager(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_sm_info *sm_info = SM_I(sbi);
+	if (!sm_info)
+		return;
 	destroy_dirty_segmap(sbi);
 	destroy_curseg(sbi);
 	destroy_free_segmap(sbi);
-- 
1.7.9.5

Re: [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager

[Gu Zheng]

On 11/06/2013 09:12 AM, Chao Yu wrote:

> A NULL point should avoid to be used in destroy_segment_manager after allocating memory fail for f2fs_sm_info.

Though without this patch it still can work well, because if it failed
to allocate f2fs_sm_info, the sit_info, free_info... all were NULL, and
the destory path(e.g. destroy_dirty_segmap) can deal with them well.
IMO, this patch is still a good catch. 

Regards,
Gu

> 
> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> ---
>  fs/f2fs/segment.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 3d4d5fc..ff363e6
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -1744,6 +1744,8 @@ static void destroy_sit_info(struct f2fs_sb_info *sbi)
>  void destroy_segment_manager(struct f2fs_sb_info *sbi)
>  {
>  	struct f2fs_sm_info *sm_info = SM_I(sbi);
> +	if (!sm_info)
> +		return;
>  	destroy_dirty_segmap(sbi);
>  	destroy_curseg(sbi);
>  	destroy_free_segmap(sbi);



------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk

Re: [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager

[Chao Yu]

Hi Gu,

> -----Original Message-----
> From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com]
> Sent: Wednesday, November 06, 2013 11:41 AM
> To: Chao Yu
> Cc: ???; linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; 谭姝
> Subject: Re: [f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager
> 
> On 11/06/2013 09:12 AM, Chao Yu wrote:
> 
> > A NULL point should avoid to be used in destroy_segment_manager after allocating memory fail for f2fs_sm_info.
> 
> Though without this patch it still can work well, because if it failed
> to allocate f2fs_sm_info, the sit_info, free_info... all were NULL, and
> the destory path(e.g. destroy_dirty_segmap) can deal with them well.

I think it could not work well. Without this patch we may got a segment 
fault in DIRTY_I(sbi) at the following code if it failed to allocate 
f2fs_sm_info memory(sbi->sm_info). Right?

static void destroy_dirty_segmap(struct f2fs_sb_info *sbi)
{
	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);

> IMO, this patch is still a good catch.
> 
> Regards,
> Gu
> 
> >
> > Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> > ---
> >  fs/f2fs/segment.c |    2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> > index 3d4d5fc..ff363e6
> > --- a/fs/f2fs/segment.c
> > +++ b/fs/f2fs/segment.c
> > @@ -1744,6 +1744,8 @@ static void destroy_sit_info(struct f2fs_sb_info *sbi)
> >  void destroy_segment_manager(struct f2fs_sb_info *sbi)
> >  {
> >  	struct f2fs_sm_info *sm_info = SM_I(sbi);
> > +	if (!sm_info)
> > +		return;
> >  	destroy_dirty_segmap(sbi);
> >  	destroy_curseg(sbi);
> >  	destroy_free_segmap(sbi);



------------------------------------------------------------------------------
November Webinars for C, C++, Fortran Developers
Accelerate application performance with scalable programming models. Explore
techniques for threading, error checking, porting, and tuning. Get the most 
from the latest Intel processors and coprocessors. See abstracts and register
http://pubads.g.doubleclick.net/gampad/clk?id=60136231&iu=/4140/ostg.clktrk
_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager

[Gu Zheng]

On 11/06/2013 01:10 PM, Chao Yu wrote:

> Hi Gu,
> 
>> -----Original Message-----
>> From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com]
>> Sent: Wednesday, November 06, 2013 11:41 AM
>> To: Chao Yu
>> Cc: ???; linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; 谭姝
>> Subject: Re: [f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager
>>
>> On 11/06/2013 09:12 AM, Chao Yu wrote:
>>
>>> A NULL point should avoid to be used in destroy_segment_manager after allocating memory fail for f2fs_sm_info.
>>
>> Though without this patch it still can work well, because if it failed
>> to allocate f2fs_sm_info, the sit_info, free_info... all were NULL, and
>> the destory path(e.g. destroy_dirty_segmap) can deal with them well.
> 
> I think it could not work well. Without this patch we may got a segment 
> fault in DIRTY_I(sbi) at the following code if it failed to allocate 
> f2fs_sm_info memory(sbi->sm_info). Right?

Yes, you're right. SIT_I generates sit_info from f2fs_sm_info.
Sorry for my mistake.:(

Regards,
Gu

> 
> static void destroy_dirty_segmap(struct f2fs_sb_info *sbi)
> {
> 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> 
>> IMO, this patch is still a good catch.
>>
>> Regards,
>> Gu
>>
>>>
>>> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
>>> ---
>>>  fs/f2fs/segment.c |    2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
>>> index 3d4d5fc..ff363e6
>>> --- a/fs/f2fs/segment.c
>>> +++ b/fs/f2fs/segment.c
>>> @@ -1744,6 +1744,8 @@ static void destroy_sit_info(struct f2fs_sb_info *sbi)
>>>  void destroy_segment_manager(struct f2fs_sb_info *sbi)
>>>  {
>>>  	struct f2fs_sm_info *sm_info = SM_I(sbi);
>>> +	if (!sm_info)
>>> +		return;
>>>  	destroy_dirty_segmap(sbi);
>>>  	destroy_curseg(sbi);
>>>  	destroy_free_segmap(sbi);
> 
> 
> 

RE: [f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager

[Chao Yu]

Hi Gu, 

Thanks for your reviewing!

Regards
Yu

> -----Original Message-----
> From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com]
> Sent: Wednesday, November 06, 2013 1:16 PM
> To: Chao Yu
> Cc: '???'; linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; '谭姝'
> Subject: Re: [f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager
> 
> On 11/06/2013 01:10 PM, Chao Yu wrote:
> 
> > Hi Gu,
> >
> >> -----Original Message-----
> >> From: Gu Zheng [mailto:guz.fnst@cn.fujitsu.com]
> >> Sent: Wednesday, November 06, 2013 11:41 AM
> >> To: Chao Yu
> >> Cc: ???; linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; linux-f2fs-devel@lists.sourceforge.net; 谭姝
> >> Subject: Re: [f2fs-dev] [PATCH] f2fs: avoid to use a NULL point in destroy_segment_manager
> >>
> >> On 11/06/2013 09:12 AM, Chao Yu wrote:
> >>
> >>> A NULL point should avoid to be used in destroy_segment_manager after allocating memory fail for f2fs_sm_info.
> >>
> >> Though without this patch it still can work well, because if it failed
> >> to allocate f2fs_sm_info, the sit_info, free_info... all were NULL, and
> >> the destory path(e.g. destroy_dirty_segmap) can deal with them well.
> >
> > I think it could not work well. Without this patch we may got a segment
> > fault in DIRTY_I(sbi) at the following code if it failed to allocate
> > f2fs_sm_info memory(sbi->sm_info). Right?
> 
> Yes, you're right. SIT_I generates sit_info from f2fs_sm_info.
> Sorry for my mistake.:(
> 
> Regards,
> Gu
> 
> >
> > static void destroy_dirty_segmap(struct f2fs_sb_info *sbi)
> > {
> > 	struct dirty_seglist_info *dirty_i = DIRTY_I(sbi);
> >
> >> IMO, this patch is still a good catch.
> >>
> >> Regards,
> >> Gu
> >>
> >>>
> >>> Signed-off-by: Chao Yu <chao2.yu@samsung.com>
> >>> ---
> >>>  fs/f2fs/segment.c |    2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> >>> index 3d4d5fc..ff363e6
> >>> --- a/fs/f2fs/segment.c
> >>> +++ b/fs/f2fs/segment.c
> >>> @@ -1744,6 +1744,8 @@ static void destroy_sit_info(struct f2fs_sb_info *sbi)
> >>>  void destroy_segment_manager(struct f2fs_sb_info *sbi)
> >>>  {
> >>>  	struct f2fs_sm_info *sm_info = SM_I(sbi);
> >>> +	if (!sm_info)
> >>> +		return;
> >>>  	destroy_dirty_segmap(sbi);
> >>>  	destroy_curseg(sbi);
> >>>  	destroy_free_segmap(sbi);
> >
> >
> >
> 
> 
> =