From patchwork Sat Oct 3 18:41:24 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paul Pluzhnikov X-Patchwork-Id: 8919 Received: (qmail 58889 invoked by alias); 3 Oct 2015 18:42:00 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 58877 invoked by uid 89); 3 Oct 2015 18:41:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, SPF_PASS, T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mail-wi0-f171.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:content-type; bh=+k7uovyN6Vdq4sMVB0/+gVQUzpFgQVoAsl66aFAwA0Q=; b=HYCvSvo9lxsJd6GmGDLedeWXGpYZ/psnMjuwu+FOstvISkw3Gyd4Oht0vgoN/pdcPt Fkdeyrfwvu/XNPQyLh7v65j7kQqBBWy3nqfsAbqY1rJ2CDFuZsn8bm9rgKGXgBSCyq/S EVRQoU6x2jS4GpRjp/tvs7M0df/havPGEybC+CLxf7fTopcZvDtgz2xMw5e8YJW9/EmB PSVqAty2kNPqjKEJ1xBU/PGvKG/CReIjMlAitI3p1w91IPtLGleHd+aZLCLWyVnDqIRL v5DSFDaY3XKXBoyc810mcj3/y8IvjQ2v30agzSPz+lR9tAK2R6I5uI1xHqUs3IHkFvag Lt7w== X-Gm-Message-State: ALoCoQlROLqRJqecMY7OmU7bL/vkvT4fl/rHsCjXqHpo3Xs8KgYRdiYxCnMSxlMgJXrGVYQke2hJ X-Received: by 10.194.240.4 with SMTP id vw4mr21259330wjc.89.1443897714982; Sat, 03 Oct 2015 11:41:54 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20150928173140.GA5374@vapier.lan> References: <20150928173140.GA5374@vapier.lan> From: Paul Pluzhnikov Date: Sat, 3 Oct 2015 11:41:24 -0700 Message-ID: Subject: Re: [patch] Fix BZ 19012 -- memory leak on error path in iconv_open To: Paul Pluzhnikov , GLIBC Devel , Paul Pluzhnikov On Mon, Sep 28, 2015 at 10:31 AM, Mike Frysinger wrote: > what about the other failure case (init_fct returns an error) ? Good point. > the strduping seems a bit convoluted and makes error handling error prone. > what about something a bit more straightforward: May as well also add a check for strdup failure. Updated patch attached. Thanks, 2015-10-03 Paul Pluzhnikov [BZ #19012] * iconv/gconv_db.c (gen_steps): Check for additional errors. Clean up on failure. diff --git a/iconv/gconv_db.c b/iconv/gconv_db.c index 4e6ec65..74b4ff5 100644 --- a/iconv/gconv_db.c +++ b/iconv/gconv_db.c @@ -243,6 +243,8 @@ gen_steps (struct derivation_step *best, const char *toset, struct __gconv_step *result; struct derivation_step *current; int status = __GCONV_NOMEM; + char *from_name = NULL; + char *to_name = NULL; /* First determine number of steps. */ for (current = best; current->last != NULL; current = current->last) @@ -259,12 +261,30 @@ gen_steps (struct derivation_step *best, const char *toset, current = best; while (step_cnt-- > 0) { - result[step_cnt].__from_name = (step_cnt == 0 - ? __strdup (fromset) - : (char *)current->last->result_set); - result[step_cnt].__to_name = (step_cnt + 1 == *nsteps - ? __strdup (current->result_set) - : result[step_cnt + 1].__from_name); + if (step_cnt == 0) + { + result[step_cnt].__from_name = from_name = __strdup (fromset); + if (from_name == NULL) + { + failed = 1; + break; + } + } + else + result[step_cnt].__from_name = (char *)current->last->result_set; + + if (step_cnt + 1 == *nsteps) + { + result[step_cnt].__to_name = to_name + = __strdup (current->result_set); + if (to_name == NULL) + { + failed = 1; + break; + } + } + else + result[step_cnt].__to_name = result[step_cnt + 1].__from_name; result[step_cnt].__counter = 1; result[step_cnt].__data = NULL; @@ -332,6 +352,8 @@ gen_steps (struct derivation_step *best, const char *toset, while (++step_cnt < *nsteps) __gconv_release_step (&result[step_cnt]); free (result); + free (from_name); + free (to_name); *nsteps = 0; *handle = NULL; if (status == __GCONV_OK) @@ -828,8 +850,9 @@ free_modules_db (struct gconv_module *node) /* Free all resources if necessary. */ libc_freeres_fn (free_mem) { - /* First free locale memory. This needs to be done before freeing derivations, - as ctype cleanup functions dereference steps arrays which we free below. */ + /* First free locale memory. This needs to be done before freeing + derivations, as ctype cleanup functions dereference steps arrays which we + free below. */ _nl_locale_subfreeres (); /* finddomain.c has similar problem. */