[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250327115756.GO1827@brightrain.aerifal.cx>
Date: Thu, 27 Mar 2025 07:57:57 -0400
From: Rich Felker <dalias@...c.org>
To: Natanael Copa <ncopa@...inelinux.org>
Cc: musl@...ts.openwall.com, Anders Svensson <anders.otp@...il.com>
Subject: Re: strerror_l() segfault
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@...c.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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
