> On Sep 26, 2013, at 10:05 PM, Li Zhong <zhong@xxxxxxxxxxxxxxxxxx> wrote:
>
>> 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>
>
Sorry, I meant that for sgi but wasn't clear. Sometimes I talk too much. :)
Eric
>>
>>> 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
