[PATCH] intl: Fix memory leak in _nl_find_domain on allocation failure

[Adhemerval Zanella Netto]

On 05/05/26 08:28, Avinal Kumar wrote:
> When _nl_explode_name() returns -1 (out of memory) and the locale was
> resolved through an alias, _nl_find_domain() returns immediately
> without freeing the locale copy allocated earlier.  Similarly,
> when _nl_make_l10nflist() returns NULL, the 'goto out' skips the
> alias_value free.
> 
> Fix by nesting the _nl_make_l10nflist() call and its result handling
> inside 'if (mask != -1)' instead of returning early.  Move the
> normalized_codeset free inside the same block.  Both failure paths
> now fall through to the unconditional alias_value free at the end.
> 
> Imported from GNU gettext commit 10eafd9e5.
> Original author: Bruno Haible <bruno@clisp.org>
> 
> Signed-off-by: Avinal Kumar <avinal.xlvii@gmail.com>


LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
> This is the second patch syncing glibc with gettext.
> 
>  intl/finddomain.c | 68 ++++++++++++++++++++++-------------------------
>  1 file changed, 32 insertions(+), 36 deletions(-)
> 
> diff --git a/intl/finddomain.c b/intl/finddomain.c
> index 7da0cce62f..c9153b8d8e 100644
> --- a/intl/finddomain.c
> +++ b/intl/finddomain.c
> @@ -50,7 +50,6 @@
>  /* List of already loaded domains.  */
>  static struct loaded_l10nfile *_nl_loaded_domains;
>  
> -
>  /* Return a data structure describing the message catalog described by
>     the DOMAINNAME and CATEGORY parameters with respect to the currently
>     established bindings.  */
> @@ -133,55 +132,52 @@ _nl_find_domain (const char *dirname, char *locale,
>  
>    /* Now we determine the single parts of the locale name.  First
>       look for the language.  Termination symbols are `_', '.', and `@'.  */
> -  mask = _nl_explode_name (locale, &language, &modifier, &territory,
> -			   &codeset, &normalized_codeset);
> -  if (mask == -1)
> -    /* This means we are out of core.  */
> -    return NULL;
> -
> -  /* We need to protect modifying the _NL_LOADED_DOMAINS data.  */
> -  gl_rwlock_wrlock (lock);
> -
> -  /* Create all possible locale entries which might be interested in
> -     generalization.  */
> -  retval = _nl_make_l10nflist (&_nl_loaded_domains, dirname,
> -			       strlen (dirname) + 1, mask, language, territory,
> -			       codeset, normalized_codeset, modifier,
> -			       domainname, 1);
> +  mask = _nl_explode_name (locale, &language, &modifier, &territory, &codeset,
> +			   &normalized_codeset);
> +  if (mask != -1) /* Not out of memory?  */
> +    {
> +      /* We need to protect modifying the _NL_LOADED_DOMAINS data.  */
> +      gl_rwlock_wrlock (lock);
>  
> -  gl_rwlock_unlock (lock);
> +      /* Create all possible locale entries which might be interested in
> +         generalization.  */
> +      retval = _nl_make_l10nflist (&_nl_loaded_domains, dirname,
> +				   strlen (dirname) + 1, mask, language,
> +				   territory, codeset, normalized_codeset,
> +				   modifier, domainname, 1);
>  
> -  if (retval == NULL)
> -    /* This means we are out of core.  */
> -    goto out;
> +      gl_rwlock_unlock (lock);
>  
> -  if (retval->decided <= 0)
> -    _nl_load_domain (retval, domainbinding);
> -  if (retval->data == NULL)
> -    {
> -      int cnt;
> -      for (cnt = 0; retval->successor[cnt] != NULL; ++cnt)
> +      if (retval != NULL) /* Not out of memory?  */
>  	{
> -	  if (retval->successor[cnt]->decided <= 0)
> -	    _nl_load_domain (retval->successor[cnt], domainbinding);
> -	  if (retval->successor[cnt]->data != NULL)
> -	    break;
> +	  if (retval->decided <= 0)
> +	    _nl_load_domain (retval, domainbinding);
> +	  if (retval->data == NULL)
> +	    {
> +	      int cnt;
> +	      for (cnt = 0; retval->successor[cnt] != NULL; ++cnt)
> +		{
> +		  if (retval->successor[cnt]->decided <= 0)
> +		    _nl_load_domain (retval->successor[cnt], domainbinding);
> +		  if (retval->successor[cnt]->data != NULL)
> +		    break;
> +		}
> +	    }
>  	}
> +
> +      /* The space for normalized_codeset is dynamically allocated.
> +         Free it.  */
> +      if (mask & XPG_NORM_CODESET)
> +	free ((void *) normalized_codeset);
>      }
>  
>    /* The room for an alias was dynamically allocated.  Free it now.  */
>    if (alias_value != NULL)
>      free (locale);
>  
> -out:
> -  /* The space for normalized_codeset is dynamically allocated.  Free it.  */
> -  if (mask & XPG_NORM_CODESET)
> -    free ((void *) normalized_codeset);
> -
>    return retval;
>  }
>  
> -
>  #ifdef _LIBC
>  /* This is called from iconv/gconv_db.c's free_mem, as locales must
>     be freed before freeing gconv steps arrays.  */