[musl] strerror_l() segfault

[musl] strerror_l() segfault

[Anders Svensson]

I recently noticed a case [1] in which zfs diff segfaults for me on
Alpine 3.21.3 (musl 1.2.5), and then noticed that it didn't seem to
have anything to do with zfs: this little test utility, reproducing
the call zfs-2.2.7 was making, segfaults on both Alpine and Void/musl:

#include <errno.h>
#include <string.h>
#include <locale.h>
#include <assert.h>

int main(int argc, char **argv) {
    locale_t loc = uselocale(0);
    assert(loc != 0);
    char *err = strerror_l(ENOENT, loc);  /* segfaults here */
    return 0;
}

I read [2] that "Locale support is very limited, and barely works", so
should I just file this failure under that heading or is strerror_l()
unexpectedly broken? There doesn't seem to be anything wrong with how
zfs is using it, but I'm no expert.

[1] https://gitlab.alpinelinux.org/alpine/aports/-/issues/16994
[2] https://wiki.musl-libc.org/open-issues

Re: [musl] strerror_l() segfault

[Rich Felker]

On Thu, Mar 20, 2025 at 11:39:17AM +0100, Anders Svensson wrote:
> I recently noticed a case [1] in which zfs diff segfaults for me on
> Alpine 3.21.3 (musl 1.2.5), and then noticed that it didn't seem to
> have anything to do with zfs: this little test utility, reproducing
> the call zfs-2.2.7 was making, segfaults on both Alpine and Void/musl:
> 
> #include <errno.h>
> #include <string.h>
> #include <locale.h>
> #include <assert.h>
> 
> int main(int argc, char **argv) {
>     locale_t loc = uselocale(0);
>     assert(loc != 0);
>     char *err = strerror_l(ENOENT, loc);  /* segfaults here */
>     return 0;
> }
> 
> I read [2] that "Locale support is very limited, and barely works", so
> should I just file this failure under that heading or is strerror_l()
> unexpectedly broken? There doesn't seem to be anything wrong with how
> zfs is using it, but I'm no expert.

If you have not set a thread-local locale with uselocale, loc is equal
to LC_GLOBAL_LOCALE, which is not a valid argument to the *_l
functions per POSIX. There's been some discussion of this and I think
the current understanding is that most (all?) other implementations do
accept LC_GLOBAL_LOCALE here, and that it's necessary to do so for the
API to actually be usable, and that POSIX needs to adopt this
requirement. So musl will probably be adding this.

Rich

Re: [musl] strerror_l() segfault

[Natanael Copa]

On Thu, 20 Mar 2025 09:47:18 -0400
Rich Felker <dalias@libc.org> wrote:

> On Thu, Mar 20, 2025 at 11:39:17AM +0100, Anders Svensson wrote:
> > I recently noticed a case [1] in which zfs diff segfaults for me on
> > Alpine 3.21.3 (musl 1.2.5), and then noticed that it didn't seem to
> > have anything to do with zfs: this little test utility, reproducing
> > the call zfs-2.2.7 was making, segfaults on both Alpine and Void/musl:
> > 
> > #include <errno.h>
> > #include <string.h>
> > #include <locale.h>
> > #include <assert.h>
> > 
> > int main(int argc, char **argv) {
> >     locale_t loc = uselocale(0);
> >     assert(loc != 0);
> >     char *err = strerror_l(ENOENT, loc);  /* segfaults here */
> >     return 0;
> > }
> > 
> > I read [2] that "Locale support is very limited, and barely works", so
> > should I just file this failure under that heading or is strerror_l()
> > unexpectedly broken? There doesn't seem to be anything wrong with how
> > zfs is using it, but I'm no expert.  
> 
> If you have not set a thread-local locale with uselocale, loc is equal
> to LC_GLOBAL_LOCALE, which is not a valid argument to the *_l
> functions per POSIX. There's been some discussion of this and I think
> the current understanding is that most (all?) other implementations do
> accept LC_GLOBAL_LOCALE here, and that it's necessary to do so for the
> API to actually be usable, and that POSIX needs to adopt this
> requirement. So musl will probably be adding this.

Would something like this work:

diff --git a/src/errno/strerror.c b/src/errno/strerror.c
index 7f926432..cea7c2ae 100644
--- a/src/errno/strerror.c
+++ b/src/errno/strerror.c
@@ -36,7 +36,7 @@ char *__strerror_l(int e, locale_t loc)
 #endif
        if (e >= sizeof errmsgidx / sizeof *errmsgidx) e = 0;
        s = (char *)&errmsgstr + errmsgidx[e];
-       return (char *)LCTRANS(s, LC_MESSAGES, loc);
+       return (char *)LCTRANS(s, LC_MESSAGES, loc == LC_GLOBAL_LOCALE ? CURRENT_LOCALE : loc);
 }
 
 char *strerror(int e)


-nc

Re: [musl] strerror_l() segfault

[Rich Felker]

On Thu, Mar 20, 2025 at 04:42:10PM +0100, Natanael Copa wrote:
> On Thu, 20 Mar 2025 09:47:18 -0400
> Rich Felker <dalias@libc.org> wrote:
> 
> > On Thu, Mar 20, 2025 at 11:39:17AM +0100, Anders Svensson wrote:
> > > I recently noticed a case [1] in which zfs diff segfaults for me on
> > > Alpine 3.21.3 (musl 1.2.5), and then noticed that it didn't seem to
> > > have anything to do with zfs: this little test utility, reproducing
> > > the call zfs-2.2.7 was making, segfaults on both Alpine and Void/musl:
> > > 
> > > #include <errno.h>
> > > #include <string.h>
> > > #include <locale.h>
> > > #include <assert.h>
> > > 
> > > int main(int argc, char **argv) {
> > >     locale_t loc = uselocale(0);
> > >     assert(loc != 0);
> > >     char *err = strerror_l(ENOENT, loc);  /* segfaults here */
> > >     return 0;
> > > }
> > > 
> > > I read [2] that "Locale support is very limited, and barely works", so
> > > should I just file this failure under that heading or is strerror_l()
> > > unexpectedly broken? There doesn't seem to be anything wrong with how
> > > zfs is using it, but I'm no expert.  
> > 
> > If you have not set a thread-local locale with uselocale, loc is equal
> > to LC_GLOBAL_LOCALE, which is not a valid argument to the *_l
> > functions per POSIX. There's been some discussion of this and I think
> > the current understanding is that most (all?) other implementations do
> > accept LC_GLOBAL_LOCALE here, and that it's necessary to do so for the
> > API to actually be usable, and that POSIX needs to adopt this
> > requirement. So musl will probably be adding this.
> 
> Would something like this work:
> 
> diff --git a/src/errno/strerror.c b/src/errno/strerror.c
> index 7f926432..cea7c2ae 100644
> --- a/src/errno/strerror.c
> +++ b/src/errno/strerror.c
> @@ -36,7 +36,7 @@ char *__strerror_l(int e, locale_t loc)
>  #endif
>         if (e >= sizeof errmsgidx / sizeof *errmsgidx) e = 0;
>         s = (char *)&errmsgstr + errmsgidx[e];
> -       return (char *)LCTRANS(s, LC_MESSAGES, loc);
> +       return (char *)LCTRANS(s, LC_MESSAGES, loc == LC_GLOBAL_LOCALE ? CURRENT_LOCALE : loc);
>  }
>  
>  char *strerror(int e)

If this it to be changed, it needs to be changed uniformly across all
the interfaces that take locale_t for which it makes sense, not just
strerror_l, and should be done with a shared macro or something. In
particular, for interfaces that just use LCTRANS, it should probably
be wrapped up in LCTRANS. I'll look at making a patch, but first we
need to establish if this is something applications expect to be
supported across implementations (do all the others already accept
LC_GLOBAL_LOCALE?).

Rich