Bug 14461 - __m128i_strloadu_tolower returns unaligned __m128i
Summary: __m128i_strloadu_tolower returns unaligned __m128i
Status: RESOLVED INVALID
Alias: None
Product: glibc
Classification: Unclassified
Component: libc (show other bugs)
Version: 2.17
: P2 critical
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-13 01:00 UTC by oblique
Modified: 2014-06-13 14:01 UTC (History)
6 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:
fweimer: security-


Attachments
patch (224 bytes, patch)
2012-08-13 01:00 UTC, oblique
Details | Diff
patch v2 (315 bytes, patch)
2012-08-13 03:37 UTC, oblique
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description oblique 2012-08-13 01:00:42 UTC
Created attachment 6575 [details]
patch

__m128i_strloadu_tolower function in sysdeps/x86_64/multiarch/strcasestr-nonascii.c does not use _mm_loadu_si128() to return the variable. So, if the caller has unaligned stack the program will crash.

We had crashes with openssl and glibc when we use Wine and the solution was to use _mm_loadu_si128().

Here are the bug reports:
https://bugs.archlinux.org/task/23277
https://bugs.archlinux.org/task/31020

I attached the patch.

Thanks.
Comment 1 oblique 2012-08-13 03:37:10 UTC
Created attachment 6576 [details]
patch v2

I decided to run Coccinelle to see if the same bug exists in other places and I found it in one more function which is a testcase.

I attached a 2nd version of the patch.
Comment 2 Andreas Jaeger 2012-08-13 18:56:34 UTC
The function returns an object of type __m128i - which is be properly aligned if the stack is properly aligned.

Looking at the arch reports, this looks related to this gcc bug:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40838

The question is should glibc work around those problems and be on the safe side - or does this need fixing in other libraries and programs?

I'm adding HJ and Jakub for discusion since both have been involved in the GCC bug.
Comment 3 oblique 2012-08-13 19:08:03 UTC
Any other functions that they return an object of type __m128i they use _mm_loadu_si128 function. e.g. the functions that are in sysdeps/x86_64/multiarch/strstr.c
Comment 4 Andreas Jaeger 2012-08-13 20:20:10 UTC
grep shows:
sysdeps/x86_64/multiarch/strcspn-c.c:	      mask = _mm_loadu_si128 ((__m128i *) a);
sysdeps/x86_64/multiarch/varshift.h:			   _mm_loadu_si128 ((__m128i *) (___m128i_shift_right
sysdeps/x86_64/multiarch/strspn-c.c:	      mask = _mm_loadu_si128 ((__m128i *) a);
sysdeps/x86_64/multiarch/strstr.c:  return _mm_loadu_si128 ((__m128i *) p);

And looking at the files, it's (except varshift.h) the input parameter that is returned. In these cases the loadu is always needed.
Comment 5 oblique 2012-08-13 21:35:56 UTC
Indeed..
I also tried __attribute__((__aligned__(16))) and still crashes. Only with _mm_loadu_si128 it did not.
Comment 6 Jakub Jelinek 2012-08-14 06:28:02 UTC
Whatever function in the backtrace misaligned the stack should be fixed.  So, if it is openssl, the fix needs to be done there, if it is some glibc routine, you need to state which one.  If you use -mpreferred-stack-boundary=2 or similar on some code, you can do that only if you don't call any code built without that option from such code.
Comment 7 Carlos O'Donell 2012-08-14 15:05:45 UTC
Waiting for submitter to provide the name of the glibc function which misaligns the stack.

I agree with Jakub, making each function robust in the face of a misaligned stack is not correct. The program must conform the ABI and keep the stack aligned as required at function call time.

If there are no glibc functions which misalign the stack then this issue should be closed as RESOLVED WONTFIX.
Comment 8 oblique 2012-08-14 15:16:39 UTC
I did some deeper debugging and it's not glibc's fault. You can close it if you want.
Comment 9 Carlos O'Donell 2012-08-14 15:24:42 UTC
Thanks! Marking RESOLVED WONTFIX.
Comment 10 Andreas Schwab 2012-08-14 16:20:10 UTC
Not a bug.