i386 TLS_INIT_TP might produce bogus asm changing stack pointer.
Commit Message
On Tue, 2014-08-26 at 14:29 -0700, Roland McGrath wrote:
> The change looks good and even seems probably appropriate to slip in during
> the release freeze.
That would be great, it does cause some head-scratching for anybody
trying to run any i386 program under valgrind with glibc 2.20.
> But, per policy, as this has user-visible effects it
> needs a bugzilla item filed.
Filed https://sourceware.org/bugzilla/show_bug.cgi?id=17319
Updated commit mentioning the bug number in the commit message and
ChangeLog entry attached.
Thanks,
Mark
Comments
That looks perfect to me. But Allan has to approve during the freeze.
On 28/08/14 02:58, Roland McGrath wrote:
> That looks perfect to me. But Allan has to approve during the freeze.
>
OK for master.
Allan
On 08/28/2014 01:01 AM, Allan McRae wrote:
> On 28/08/14 02:58, Roland McGrath wrote:
>> That looks perfect to me. But Allan has to approve during the freeze.
>>
>
> OK for master.
Thanks. I added the bug number to NEWS and pushed this on behalf of
Mark because he hasn't got commit access.
From ab4888b207f07fd762c485f94f9543b0ed456a4d Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Wed, 27 Aug 2014 17:07:58 +0200
Subject: [PATCH] i386 TLS_INIT_TP might produce bogus asm changing stack
pointer [BZ #17319]
TLS_INIT_TP in sysdeps/i386/nptl/tls.h uses some hand written asm to
generate a set_thread_area that might result in exchanging ebx and esp
around the syscall causing introspection tools like valgrind to loose
track of the user stack. Just use INTERNAL_SYSCALL which makes sure
esp isn't changed arbitrarily.
Before the patch the code would generate:
mov $0xf3,%eax
movl $0xfffff,0x8(%esp)
movl $0x51,0xc(%esp)
xchg %esp,%ebx
int $0x80
xchg %esp,%ebx
Using INTERNAL_SYSCALL instead will generate:
movl $0xfffff,0x8(%esp)
movl $0x51,0xc(%esp)
xchg %ecx,%ebx
mov $0xf3,%eax
int $0x80
xchg %ecx,%ebx
Thanks to Florian Weimer for analysing why the original code generated
the bogus esp usage:
_segdescr.desc happens to be at the top of the stack, so its address
is in %esp. The asm statement says that %3 is an input, so its value
will not change, and GCC can use %esp as the input register for the
expression &_segdescr.desc. But the constraints do not fully describe
the asm statement because the %3 register is actually modified, albeit
only temporarily.
[BZ #17319]
* sysdeps/i386/nptl/tls.h (TLS_INIT_TP): Use INTERNAL_SYSCALL
to call set_thread_area instead of hand written asm.
(__NR_set_thread_area): Removed define.
(TLS_FLAG_WRITABLE): Likewise.
(__ASSUME_SET_THREAD_AREA): Remove check.
(TLS_EBX_ARG): Remove define.
(TLS_LOAD_EBX): Likewise.
---
ChangeLog | 11 +++++++++++
sysdeps/i386/nptl/tls.h | 31 ++-----------------------------
2 files changed, 13 insertions(+), 29 deletions(-)
@@ -1,3 +1,14 @@
+2014-08-27 Mark Wielaard <mjw@redhat.com>
+
+ [BZ #17319]
+ * sysdeps/i386/nptl/tls.h (TLS_INIT_TP): Use INTERNAL_SYSCALL
+ to call set_thread_area instead of hand written asm.
+ (__NR_set_thread_area): Removed define.
+ (TLS_FLAG_WRITABLE): Likewise.
+ (__ASSUME_SET_THREAD_AREA): Remove check.
+ (TLS_EBX_ARG): Remove define.
+ (TLS_LOAD_EBX): Likewise.
+
2014-08-27 Allan McRae <allan@archlinux.org>
* sysdeps/i386/fpu/libm-test-ulps: Update ULPs.
@@ -154,29 +154,6 @@ union user_desc_init
__asm ("movw %w0, %%gs" :: "q" (val))
# endif
-
-# ifndef __NR_set_thread_area
-# define __NR_set_thread_area 243
-# endif
-# ifndef TLS_FLAG_WRITABLE
-# define TLS_FLAG_WRITABLE 0x00000001
-# endif
-
-// XXX Enable for the real world.
-#if 0
-# ifndef __ASSUME_SET_THREAD_AREA
-# error "we need set_thread_area"
-# endif
-#endif
-
-# ifdef __PIC__
-# define TLS_EBX_ARG "r"
-# define TLS_LOAD_EBX "xchgl %3, %%ebx\n\t"
-# else
-# define TLS_EBX_ARG "b"
-# define TLS_LOAD_EBX
-# endif
-
#if defined NEED_DL_SYSINFO
# define INIT_SYSINFO \
_head->sysinfo = GLRO(dl_sysinfo)
@@ -231,12 +208,8 @@ tls_fill_user_desc (union user_desc_init *desc,
tls_fill_user_desc (&_segdescr, -1, _thrdescr); \
\
/* Install the TLS. */ \
- asm volatile (TLS_LOAD_EBX \
- "int $0x80\n\t" \
- TLS_LOAD_EBX \
- : "=a" (_result), "=m" (_segdescr.desc.entry_number) \
- : "0" (__NR_set_thread_area), \
- TLS_EBX_ARG (&_segdescr.desc), "m" (_segdescr.desc)); \
+ INTERNAL_SYSCALL_DECL (err); \
+ _result = INTERNAL_SYSCALL (set_thread_area, err, 1, &_segdescr.desc); \
\
if (_result == 0) \
/* We know the index in the GDT, now load the segment register. \
--
1.9.3