fallback to no translations when gettext("")

fallback to no translations when gettext("")

[He X]

[-- Attachment #1.1: Type: text/plain, Size: 475 bytes --]

following http://www.openwall.com/lists/musl/2017/02/13/8 ,
http://www.openwall.com/lists/musl/2017/03/04/1 and
http://www.openwall.com/lists/musl/2017/02/13/5:

> gnu tar showed me segfaults, because he passed a zero msgid1 causing
__mo_lookup segfault, we should add a check in dcngettext to avoid it(if
(!msgid1) goto notrans;):

 #2  0x00007ffff7d82a6f in dcngettext (domainname=0x6737a0 "tar",
msgid1=0x0, msgid2=0x0, n=1,
    category=5) at src/locale/dcngettext.c:211

[-- Attachment #1.2: Type: text/html, Size: 795 bytes --]

[-- Attachment #2: locale.diff --]
[-- Type: text/plain, Size: 461 bytes --]

--- musl-1.1.16/src/locale/dcngettext.c	2017-03-04 08:05:45.755115338 +0000
+++ musl-1.1.16/src/locale/dcngettext.c	2017-03-04 08:05:45.755115338 +0000
@@ -121,6 +121,7 @@
 char *dcngettext(const char *domainname, const char *msgid1, const char *msgid2, unsigned long int n, int category)
 {
+	if (!msgid1) goto notrans;
 	static struct msgcat *volatile cats;
 	struct msgcat *p;
 	struct __locale_struct *loc = CURRENT_LOCALE;
 	const struct __locale_map *lm;

Re: fallback to no translations when gettext("")

[He X]

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

Hi, rich, happy that you merged the changes related to dcngettext(). But
you seems forget this simple patch and did not get it with those changes.
So i bump it again now. Hope you do not mind if you still remember it.

He X

2017-03-04 16:08 GMT+08:00 He X <xw897002528@gmail.com>:

> following http://www.openwall.com/lists/musl/2017/02/13/8 ,
> http://www.openwall.com/lists/musl/2017/03/04/1 and
> http://www.openwall.com/lists/musl/2017/02/13/5:
>
> > gnu tar showed me segfaults, because he passed a zero msgid1 causing
> __mo_lookup segfault, we should add a check in dcngettext to avoid it(if
> (!msgid1) goto notrans;):
>
>  #2  0x00007ffff7d82a6f in dcngettext (domainname=0x6737a0 "tar",
> msgid1=0x0, msgid2=0x0, n=1,
>     category=5) at src/locale/dcngettext.c:211
>
>
>

[-- Attachment #2: Type: text/html, Size: 1461 bytes --]

Re: Re: fallback to no translations when gettext("")

[Rich Felker]

On Thu, Mar 23, 2017 at 01:05:55PM +0800, He X wrote:
> Hi, rich, happy that you merged the changes related to dcngettext(). But
> you seems forget this simple patch and did not get it with those changes.
> So i bump it again now. Hope you do not mind if you still remember it.
> 
> He X
> 
> 2017-03-04 16:08 GMT+08:00 He X <xw897002528@gmail.com>:
> 
> > following http://www.openwall.com/lists/musl/2017/02/13/8 ,
> > http://www.openwall.com/lists/musl/2017/03/04/1 and
> > http://www.openwall.com/lists/musl/2017/02/13/5:
> >
> > > gnu tar showed me segfaults, because he passed a zero msgid1 causing
> > __mo_lookup segfault, we should add a check in dcngettext to avoid it(if
> > (!msgid1) goto notrans;):
> >
> >  #2  0x00007ffff7d82a6f in dcngettext (domainname=0x6737a0 "tar",
> > msgid1=0x0, msgid2=0x0, n=1,
> >     category=5) at src/locale/dcngettext.c:211

Indeed, I did forget. But looking at it more closely, according to the
GNU gettext manual I think this is a bug in GNU tar:

    "If the argument is NULL the result is undefined."

https://www.gnu.org/software/gettext/manual/html_node/Interface-to-gettext.html

Whether we commit this patch or not, I think it should be reported as
a bug in GNU tar.

Given that the API documentation says a NULL pointer is not valid
here, my leaning, in alignment with existing musl practice, is _not_
to make it silently "work" but to leave it crashing, so that this sort
of bug is caught. But if the documentation is wrong and the practice
of passing null pointers here is widely accepted we can reconsider.

Rich

Re: Re: fallback to no translations when gettext("")

[He X]

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

I tested a full-symbol version and found that tar segfaulted due to 'argp'.
Tar is using its own internal implementation of argp, instead of linking to
the one most musl-based distros are using - argp-standalone. In other
words, that internal implementation is not compatible with musl and there's
no actual bug in tar.

I am satisfied with add -largp, but one more question: what's the reason
for not adding argp to musl? I can understand execinfo is hard to add (from
mail)and fts is waiting for changes of API(in wiki). They're not portable
for adding, so what's wrong with argp then? I did not find any mail related
to this question. And I am sorry if i missed anything.

He X

2017-03-25 2:36 GMT+08:00 Rich Felker <dalias@libc.org>:

> On Thu, Mar 23, 2017 at 01:05:55PM +0800, He X wrote:
> > Hi, rich, happy that you merged the changes related to dcngettext(). But
> > you seems forget this simple patch and did not get it with those changes.
> > So i bump it again now. Hope you do not mind if you still remember it.
> >
> > He X
> >
> > 2017-03-04 16:08 GMT+08:00 He X <xw897002528@gmail.com>:
> >
> > > following http://www.openwall.com/lists/musl/2017/02/13/8 ,
> > > http://www.openwall.com/lists/musl/2017/03/04/1 and
> > > http://www.openwall.com/lists/musl/2017/02/13/5:
> > >
> > > > gnu tar showed me segfaults, because he passed a zero msgid1 causing
> > > __mo_lookup segfault, we should add a check in dcngettext to avoid
> it(if
> > > (!msgid1) goto notrans;):
> > >
> > >  #2  0x00007ffff7d82a6f in dcngettext (domainname=0x6737a0 "tar",
> > > msgid1=0x0, msgid2=0x0, n=1,
> > >     category=5) at src/locale/dcngettext.c:211
>
> Indeed, I did forget. But looking at it more closely, according to the
> GNU gettext manual I think this is a bug in GNU tar:
>
>     "If the argument is NULL the result is undefined."
>
> https://www.gnu.org/software/gettext/manual/html_node/
> Interface-to-gettext.html
>
> Whether we commit this patch or not, I think it should be reported as
> a bug in GNU tar.
>
> Given that the API documentation says a NULL pointer is not valid
> here, my leaning, in alignment with existing musl practice, is _not_
> to make it silently "work" but to leave it crashing, so that this sort
> of bug is caught. But if the documentation is wrong and the practice
> of passing null pointers here is widely accepted we can reconsider.
>
> Rich
>

[-- Attachment #2: Type: text/html, Size: 3582 bytes --]