On Thu, 2013-09-26 at 09:31 -0500, Eric Sandeen wrote:
> On 9/26/13 1:45 AM, Li Zhong wrote:
> > If verify_set_primary_sb() completes the secondary sb scanning loop with
> > too few valid secondaries found (num_ok < num_sbs / 2), it will immediately
> > return without freeing any of the previously allocated memory (variables
> > sb, checked, and any items on the geo list). This was reported by
> > the Coverity scanner as CID 997012, 997013 and 997014.
> >
> > Fix this by using the out_free_list: goto target for this error case.
> >
> > Earlier, if get_sb() fails in the secondary scan loop, it goes to
> > the out: target which does not free any items on the geo list. Fix
> > this by using the out_free_list: target as well, and remove the now-unused
> > out: target.
> >
> > Signed-off-by: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>
> > ---
> > v2: as Mark pointed out, out in the for loop before also needs list to
> > be freed. Also remove out lable as it is not referenced any more.
> > v3: use a meaningful changlog from Eric, and hide the patch changlogs below "---".
>
> Thanks for that; you can add my:
>
> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>
> alongside Mark's.
Ah, I missed that. Is it ok to just add it here in this mail?
Reviewed-by: Mark Tinguely <tinguely@xxxxxxx>
>
> > repair/sb.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/repair/sb.c b/repair/sb.c
> > index aa550e3..d34d7a2 100644
> > --- a/repair/sb.c
> > +++ b/repair/sb.c
> > @@ -733,7 +733,7 @@ verify_set_primary_sb(xfs_sb_t *rsb,
> >
> > if (get_sb(sb, off, size, agno) == XR_EOF) {
> > retval = 1;
> > - goto out;
> > + goto out_free_list;
> > }
> >
> > if (verify_sb(sb, 0) == XR_OK) {
> > @@ -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;
> > + }
> >
> > current = get_best_geo(list);
> >
> > @@ -841,7 +843,6 @@ verify_set_primary_sb(xfs_sb_t *rsb,
> >
> > out_free_list:
> > free_geo(list);
> > -out:
> > free(sb);
> > free(checked);
> > return(retval);
> >
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
