Re: [PATCH] xfsprogs: fix potential memory leak in verify_set_primary_sb()

[Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>]

On Tue, 2013-09-24 at 13:59 -0500, Mark Tinguely wrote:
> On 09/22/13 01:01, Li Zhong wrote:
> > This patch tries to fix CID 997012, 997013 and 997014 reported by Coverity 
> > scan,
> > as suggested by sekharan.
> >
> > Signed-off-by: Li Zhong<zhong@xxxxxxxxxxxxxxxxxx>
> > ---
> 
> 
> > @@ -756,8 +756,10 @@ verify_set_primary_sb(xfs_sb_t         *rsb,
> >     /*
> >      * see if we have enough superblocks to bother with
> >      */
> > -   if (num_ok < num_sbs / 2)
> > -           return(XR_INSUFF_SEC_SB);
> > +   if (num_ok < num_sbs / 2) {
> > +           retval = XR_INSUFF_SEC_SB;
> > +           goto out_free_list;
> > +   }
> 
> >
> 
> Looks good. list, sb and check could have been allocated at this point.
> 
> Isn't the list been added to before the conditional in the for loop?:

See, I just sent a new version including below change.

Seems it is a Coverity scan bug not discovering this :)

Thanks, Zhong

> 
>          list = add_geo(list, &geo, sb_index);
> 
>          /*
>           * grab N secondaries.  check them off as we get them
>           * so we only process each one once
>           */
>          for (round = 0; round < skip; round++)  {
> 
> ...
>                          if (get_sb(sb, off, size, agno) == XR_EOF)  {
>                                  retval = 1;
>                                  goto out;
>                               ^^^^^^^^^ out_free_list?
>                          }
> 
> --Mark.
>