xfs
[Top] [All Lists]

[PATCH v3 1/2] xfsprogs: fix potential memory leak in verify_set_primary

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: [PATCH v3 1/2] xfsprogs: fix potential memory leak in verify_set_primary_sb()
From: Li Zhong <zhong@xxxxxxxxxxxxxxxxxx>
Date: Thu, 26 Sep 2013 14:45:32 +0800
Cc: Mark Tinguely <tinguely@xxxxxxx>, Chandra Seetharaman <sekharan@xxxxxxxxxx>, xfsprogs <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <5242F31B.4060902@xxxxxxxxxxx>
References: <1379829679.4089.2.camel@ThinkPad-T5421> <5241E125.7010902@xxxxxxx> <1380094327.2526.5.camel@ThinkPad-T5421> <5242F31B.4060902@xxxxxxxxxxx>
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 
"---".

 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);
-- 
1.8.1.4



<Prev in Thread] Current Thread [Next in Thread>