[PATCH, newlib/libc/locale] Fix broken '_duplocale_r' which called '__loadlocale' in an incorrect way

[PATCH, newlib/libc/locale] Fix broken '_duplocale_r' which called '__loadlocale' in an incorrect way

[Koichi MURASE]

[-- Attachment #1: Type: text/plain, Size: 2179 bytes --]

Hello, I send a patch here.

Problem:

  After  passing  locales  created  by  'duplocale'   to   'uselocale',
  referencing   'MB_CUR_MAX',   which   is   actually   expanded    to
  '__locale_mb_cur_max()' by preprocessors, causes segmentation faults.
  Direct use of locales from 'newlocale' does not  cause  the  problem.
  This is the problem of 'duplocale'.

  $ echo $LANG
  ja_JP.UTF-8
  $ cat test.c
  #include <stdlib.h>
  #include <locale.h>

  volatile int var;

  int main(void) {
    locale_t const loc = newlocale(LC_ALL_MASK, "", NULL);
    locale_t const dup = duplocale(loc);
    locale_t const old = uselocale(dup);
    var = MB_CUR_MAX; /* <-- crashes here */
    uselocale(old);
    freelocale(dup);
    freelocale(loc);
    return 0;
  }
  $ gcc test.c
  $ ./a
  Segmentation fault (core dumped)

  # Note: "core dumped" in the above message was  actually written  in
  # Japanese, but I translated the part to post a mail in English.

Bug:

  In the beginning of '__loadlocale' (newlib/libc/locale/locale.c:501),
  there is a code which checks if the operations can be skipped:

  > /* Avoid doing everything twice if nothing has changed. */
  > if (!strcmp (new_locale, loc->categories[category]))
  >   return loc->categories[category];

  While,   in   the   function   '_duplocale_r'    (newlib/libc/locale/
  duplocale.c), '__loadlocale'  is  called  as  in  the  quoted  codes:

  > /* If the object is not a "C" locale category, copy it.  Just call
  >    __loadlocale.  It knows what to do to replicate the category. */
  > tmp_locale.lc_cat[i].ptr = NULL;
  > tmp_locale.lc_cat[i].buf = NULL;
  > if (!__loadlocale (&tmp_locale, i, tmp_locale.categories[i]))
  >   goto error;

  This call of '__loadlocale' results in the skip check being

    !strcmp(tmp_locale.categories[i], tmp_locale.categories[i]),

  which is always true. This  means  that  the  actual  operations  of
  '__loadLocale' will never be performed for 'duplocale'.

Fix:

  The call of '__loadlocale' in '_duplocale_r' is modified.

  Please see the attached file for the fix as tabs in  the  mail  body
  seem to be converted to spaces by Gmail.

Best regards,
Koichi Murase

[-- Attachment #2: 0001-Fix-duplocale-libc-locale-duplocale.c-which-fails-to.patch --]
[-- Type: application/octet-stream, Size: 2986 bytes --]

From b9464ff4e4a6c0e93c1080c2f3ee8cc7d4d5d8db Mon Sep 17 00:00:00 2001
From: Koichi Murase <myoga.murase@gmail.com>
Date: Sun, 12 Mar 2017 01:27:26 +0900
Subject: [PATCH] Fix duplocale (libc/locale/duplocale.c) which fails to
 properly call __loadlocale

Problem:

  After  passing  locales  created  by  'duplocale'   to   'uselocale',
  referencing   'MB_CUR_MAX',   which   is   actually   expanded    to
  '__locale_mb_cur_max()' by preprocessors, causes segmentation faults.
  Direct use of locales from 'newlocale' does not  cause  the  problem.
  This is the problem of 'duplocale'.

  $ echo $LANG
  ja_JP.UTF-8
  $ cat test.c
  #include <stdlib.h>
  #include <locale.h>

  volatile int var;

  int main(void) {
    locale_t const loc = newlocale(LC_ALL_MASK, "", NULL);
    locale_t const dup = duplocale(loc);
    locale_t const old = uselocale(dup);
    var = MB_CUR_MAX; /* <-- crashes here */
    uselocale(old);
    freelocale(dup);
    freelocale(loc);
    return 0;
  }
  $ gcc test.c
  $ ./a
  Segmentation fault (core dumped)

  # Note: "core dumped" in the above message was  actually written  in
  # Japanese, but I translated the part to post a mail in English.

Bug:

  In the beginning of '__loadlocale' (newlib/libc/locale/locale.c:501),
  there is a code which checks if the operations can be skipped:

  > /* Avoid doing everything twice if nothing has changed. */
  > if (!strcmp (new_locale, loc->categories[category]))
  >   return loc->categories[category];

  While,   in   the   function   '_duplocale_r'    (newlib/libc/locale/
  duplocale.c), '__loadlocale'  is  called  as  in  the  quoted  codes:

  > /* If the object is not a "C" locale category, copy it.  Just call
  >    __loadlocale.  It knows what to do to replicate the category. */
  > tmp_locale.lc_cat[i].ptr = NULL;
  > tmp_locale.lc_cat[i].buf = NULL;
  > if (!__loadlocale (&tmp_locale, i, tmp_locale.categories[i]))
  >   goto error;

  This call of '__loadlocale' results in the skip check being

    !strcmp(tmp_locale.categories[i], tmp_locale.categories[i]),

  which is always true. This  means  that  the  actual  operations  of
  '__loadLocale' will never be performed for 'duplocale'.

Fix:

  The call of '__loadlocale' in '_duplocale_r' is modified.
---
 newlib/libc/locale/duplocale.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/newlib/libc/locale/duplocale.c b/newlib/libc/locale/duplocale.c
index 06ebfcd..cc1a1d5 100644
--- a/newlib/libc/locale/duplocale.c
+++ b/newlib/libc/locale/duplocale.c
@@ -64,7 +64,8 @@ _duplocale_r (struct _reent *p, struct __locale_t *locobj)
 	   __loadlocale.  It knows what to do to replicate the category. */
 	tmp_locale.lc_cat[i].ptr = NULL;
 	tmp_locale.lc_cat[i].buf = NULL;
-	if (!__loadlocale (&tmp_locale, i, tmp_locale.categories[i]))
+	tmp_locale.categories[i][0] = '\0';
+	if (!__loadlocale (&tmp_locale, i, locobj->categories[i]))
 	  goto error;
       }
 #endif /* __HAVE_LOCALE_INFO__ */
-- 
2.8.3

Re: [PATCH, newlib/libc/locale] Fix broken '_duplocale_r' which called '__loadlocale' in an incorrect way

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1956 bytes --]

On Mar 12 01:50, Koichi MURASE wrote:
> Hello, I send a patch here.
> 
> Problem:
> 
>   After  passing  locales  created  by  'duplocale'   to   'uselocale',
>   referencing   'MB_CUR_MAX',   which   is   actually   expanded    to
>   '__locale_mb_cur_max()' by preprocessors, causes segmentation faults.
>   Direct use of locales from 'newlocale' does not  cause  the  problem.
>   This is the problem of 'duplocale'.
> [...]
> Bug:
> 
>   In the beginning of '__loadlocale' (newlib/libc/locale/locale.c:501),
>   there is a code which checks if the operations can be skipped:
> 
>   > /* Avoid doing everything twice if nothing has changed. */
>   > if (!strcmp (new_locale, loc->categories[category]))
>   >   return loc->categories[category];
> 
>   While,   in   the   function   '_duplocale_r'    (newlib/libc/locale/
>   duplocale.c), '__loadlocale'  is  called  as  in  the  quoted  codes:
> 
>   > /* If the object is not a "C" locale category, copy it.  Just call
>   >    __loadlocale.  It knows what to do to replicate the category. */
>   > tmp_locale.lc_cat[i].ptr = NULL;
>   > tmp_locale.lc_cat[i].buf = NULL;
>   > if (!__loadlocale (&tmp_locale, i, tmp_locale.categories[i]))
>   >   goto error;
> 
>   This call of '__loadlocale' results in the skip check being
> 
>     !strcmp(tmp_locale.categories[i], tmp_locale.categories[i]),
> 
>   which is always true. This  means  that  the  actual  operations  of
>   '__loadLocale' will never be performed for 'duplocale'.
> 
> Fix:
> 
>   The call of '__loadlocale' in '_duplocale_r' is modified.
> 
>   Please see the attached file for the fix as tabs in  the  mail  body
>   seem to be converted to spaces by Gmail.
> 
> Best regards,
> Koichi Murase

Thanks for tracking this down and fixing.  I applied your patch with
just a bit of code commenting added.


Thanks again,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]