From patchwork Fri Sep 16 23:41:13 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Theodore Ts'o X-Patchwork-Id: 115065 Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 8F15FB70FE for ; Sat, 17 Sep 2011 09:41:18 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755212Ab1IPXlR (ORCPT ); Fri, 16 Sep 2011 19:41:17 -0400 Received: from li9-11.members.linode.com ([67.18.176.11]:52457 "EHLO test.thunk.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752291Ab1IPXlQ (ORCPT ); Fri, 16 Sep 2011 19:41:16 -0400 Received: from root (helo=tytso-glaptop.cam.corp.google.com) by test.thunk.org with local-esmtp (Exim 4.69) (envelope-from ) id 1R4i1z-0000BS-3z; Fri, 16 Sep 2011 23:41:15 +0000 Received: from tytso by tytso-glaptop.cam.corp.google.com with local (Exim 4.71) (envelope-from ) id 1R4i1x-0006Av-GY; Fri, 16 Sep 2011 19:41:13 -0400 Date: Fri, 16 Sep 2011 19:41:13 -0400 From: Ted Ts'o To: Eric Sandeen Cc: linux-ext4@vger.kernel.org Subject: Re: [PATCH 21/25] e2fsck: Fix leaks in error paths Message-ID: <20110916234113.GY16246@thunk.org> References: <1316206180-6375-1-git-send-email-sandeen@redhat.com> <1316206180-6375-22-git-send-email-sandeen@redhat.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1316206180-6375-22-git-send-email-sandeen@redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: tytso@thunk.org X-SA-Exim-Scanned: No (on test.thunk.org); SAEximRunCond expanded to false Sender: linux-ext4-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-ext4@vger.kernel.org On Fri, Sep 16, 2011 at 03:49:36PM -0500, Eric Sandeen wrote: > fn and/or array was not freed in some error paths. > > Signed-off-by: Eric Sandeen Applied, but I did make a two changes. > @@ -345,6 +346,7 @@ profile_init(const char **files, profile_t *ret_profile) > * If all the files were not found, return the appropriate error. > */ > if (!profile->first_file) { > + free_list(array); > profile_release(profile); > return ENOENT; > } I changed this to be: if (!profile->first_file) { retval = ENOENT; goto errout; } Which allows us to unify the error cleanup path and avoid duplicating code. Also, I noticed another bug while I was validating your patch. If the realloc() in get_dirlist() fails due to a ENOMEM, and we jump to errout, the array hasn't been null terminated yet. This could lead to lead a kernel oops or worse when free_list(array) tries to free the array. So I added: errout: + if (array) + array[num] = 0; to take care of this problem. - Ted commit 04bfa75f42a5adb3510551f4d153526d94be37fb Author: Eric Sandeen Date: Fri Sep 16 15:49:36 2011 -0500 e2fsck: Fix leaks in error paths fn and/or array was not freed in some error paths. [ Also make sure the array is NULL terminated before we free it in get_dirlist(). --tytso] Signed-off-by: Eric Sandeen Signed-off-by: Theodore Ts'o --- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/e2fsck/profile.c b/e2fsck/profile.c index 327bfb4..f4267b1 100644 --- a/e2fsck/profile.c +++ b/e2fsck/profile.c @@ -276,6 +276,7 @@ static errcode_t get_dirlist(const char *dirname, char***ret_array) new_array = realloc(array, sizeof(char *) * (max+1)); if (!new_array) { retval = ENOMEM; + free(fn); goto errout; } array = new_array; @@ -290,6 +291,8 @@ static errcode_t get_dirlist(const char *dirname, char***ret_array) closedir(dir); return 0; errout: + if (array) + array[num] = 0; closedir(dir); free_list(array); return retval; @@ -345,8 +348,8 @@ profile_init(const char **files, profile_t *ret_profile) * If all the files were not found, return the appropriate error. */ if (!profile->first_file) { - profile_release(profile); - return ENOENT; + retval = ENOENT; + goto errout; } }