This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] locale: don't crash if locale-archive is an empty file
- From: Aurelien Jarno <aurelien at aurel32 dot net>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Sun, 1 Dec 2013 12:23:04 +0100
- Subject: Re: [PATCH] locale: don't crash if locale-archive is an empty file
- Authentication-results: sourceware.org; auth=none
- References: <1385739561-27038-1-git-send-email-aurelien at aurel32 dot net> <5298C293 dot 4030503 at redhat dot com> <20131129165804 dot GB4601 at hall dot aurel32 dot net> <5298EC80 dot 1000408 at redhat dot com>
On Fri, Nov 29, 2013 at 02:35:28PM -0500, Carlos O'Donell wrote:
> On 11/29/2013 11:58 AM, Aurelien Jarno wrote:
> > On Fri, Nov 29, 2013 at 11:36:35AM -0500, Carlos O'Donell wrote:
> >> On 11/29/2013 10:39 AM, Aurelien Jarno wrote:
> >>> In case of power failure followed by filesystem issues locale-archive
> >>> can end-up containing all zeros. In that case all calls to setlocale()
> >>> generate a SIGFPE. This renders a system with a default non-C locale
> >>> unbootable.
> >>>
> >>> Avoid this by ignoring the locale instead of generating a SIGFPE.
> >>> ---
> >>> locale/loadarchive.c | 4 ++++
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> 2013-11-29 Aurelien Jarno <aurelien@aurel32.net>
> >>>
> >>> * locale/loadarchive.c (_nl_load_locale_from_archive): Avoid
> >>> division by 0.
> >>>
> >>> diff --git a/locale/loadarchive.c b/locale/loadarchive.c
> >>> index 70136dc..7cfc498 100644
> >>> --- a/locale/loadarchive.c
> >>> +++ b/locale/loadarchive.c
> >>> @@ -274,6 +274,10 @@ _nl_load_locale_from_archive (int category, const char **namep)
> >>> namehashtab = (struct namehashent *) ((char *) head
> >>> + head->namehash_offset);
> >>>
> >>> + /* Avoid division by 0 if the file is corrupted. */
> >>> + if (__builtin_expect (head->namehash_size == 0, 0))
> >>> + goto close_and_out;
> >>> +
> >>> idx = hval % head->namehash_size;
> >>> incr = 1 + hval % (head->namehash_size - 2);
> >>>
> >>>
> >>
> >> How did you test this?
> >
> > I fetched a broken locale-archive from [1] and set the environment
> > variables LANG="en_IN" and LANGUAGE="en_IN:en" (though I have been able
> > to reproduce with my default locales setting). Then all coreutils
> > commands started to SIGFPE (ls, mount, su, grep, etc.).
> >
> > In installed the fixed version of the libc, and all commands started
> > to work again.
>
> That's a pretty big test with a lot of variables.
>
> Could you please test this with a small program that calls setlocale
> for both a non-builtin and builtin locale?
I have now tested this by using this small code:
| #include <locale.h>
|
| int main()
| {
| setlocale(LC_ALL, "non-builtin locale");
| // setlocale(LC_ALL, "C");
| return 0;
| }
Without the patch, I get a SIGFPE with "non-builtin locale", but not
with the C locale. With the patch, both works.
> OK to checkin given testing with a smaller more concise test that
> proves there aren't any other factors involved here.
As pointed out my Mike and Adam, I should have used __glibc_unlikely, I
am therefore going to send a new version of this patch.
Cheers,
Aurelien
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net