Re: [RFC] newlib/libc/include/langinfo.h: nl_langinfo enum off by one error causing pointer overwrite

On Aug 22 15:25, Brian Inglis wrote:
> On 2024-08-22 13:13, Corinna Vinschen wrote:
> > On Aug 22 11:14, Brian Inglis wrote:
> > > As a result, the underlying pointers in the pointer array are off by one in
> > > the wide messages fields, and the _NL_COLLATE_CODESET pointer clobbers what
> > > was stored in the _NL_MESSAGES_WNOSTR position in the pointer array.
> > 
> > Yes, youre' right, there's an off by one.  But this can't be fixed by
> > chaning nl_item values.
> > 
> > The culprit is actually in nl_langinfo.c.  The internal nl_ext array
> > erronously contains an entry for lc_monetary_T::codeset which is not
> > part of the _NL_LOCALE_EXTENDED entries.  This moves the wide char
> > yes/no strings accidentally by one.  The patch is simple:
> > 
> > diff --git a/newlib/libc/locale/nl_langinfo.c b/newlib/libc/locale/nl_langinfo.c
> > index c34a7d131376..4477d833bec1 100644
> > --- a/newlib/libc/locale/nl_langinfo.c
> > +++ b/newlib/libc/locale/nl_langinfo.c
> > @@ -160,7 +160,6 @@ static struct _nl_item_t
> >     _NLITEM (monetary, wmon_thousands_sep),
> >     _NLITEM (monetary, wpositive_sign),
> >     _NLITEM (monetary, wnegative_sign),
> > -  _NLITEM (messages, codeset),
> >     _NLITEM (messages, wyesexpr),
> >     _NLITEM (messages, wnoexpr),
> >     _NLITEM (messages, wyesstr),
> > 
> > > If __HAVE_LOCALE_INFO__ is not defined, then _NL_MESSAGES_CODESET is
> > > also not defined, so it is unclear if _NL_MESSAGES_CODESET should be
> > > defined to _NL_CTYPE_CODESET if neither __HAVE_LOCALE_INFO__ nor
> > > __HAVE_LOCALE_INFO_EXTENDED__ are defined, or added as another field
> > > depending on those definitions?
> > 
> > No, see above.  There is no definition of _NL_CTYPE_CODESET, only
> > _NL_CTYPE_CODESET_NAME == CODESET.  _NL_MESSAGES_CODESET is exactly
> > where it has to be. If you start moving _NL_* values around, you *will*
> > break backward compatibility with existing executables.
> > [...]
> 
> But aren't those entries populated from Windows, where the codeset is copied in
> winsup/cygwin/nlsfuncs.cc(__set_lc_messages_from_win)
> 
> https://cygwin.com/git?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/nlsfuncs.cc#l1114
> 
> into newlib/libc/locale/setlocale.h(lc_messages_t):
> 
> https://cygwin.com/git?p=newlib-cygwin.git;a=blob;f=newlib/libc/locale/setlocale.h#l166
> 
> which includes codeset?

No.  First of all, apart from fetching LANG from the windows locale
at startup (see /etc/profile.d/lang.*), the codeset is maintained by
Cygwin/newlib alone, via LC_* environment and setlocale() calls in
the application.

Also, Windows has no GetLocaleInfo call for fetching locale-specific
yes/no strings.  I. e., there's no equivalent for LC_MESSAGES.

Therefore Cygwin fetches the information from static information
maintained in winsup/cygwin/local_includes/lc_msg.h.

If you look closely into __set_lc_messages_from_win(), you'll
notice that the __get_rfc5646_from_locale() function is called
from __set_lc_messages_from_win() *only* to check if a matching
Windows locale exists.  This is a pure sanity check.  After that,
any further information is taken from the lc_msg struct defined
in lc_msg.h, which in turn has been fetched from Linux.  See
winsup/cygwin/linux-locale-helpers.

Does that make sense?


Thanks,
Corinna