This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] i386 TLS_INIT_TP might produce bogus asm changing stack pointer.
- From: Mark Wielaard <mjw at redhat dot com>
- To: Roland McGrath <roland at hack dot frob dot com>
- Cc: Florian Weimer <fweimer at redhat dot com>, libc-alpha at sourceware dot org, Siddhesh Poyarekar <siddhesh at redhat dot com>
- Date: Wed, 27 Aug 2014 17:12:09 +0200
- Subject: Re: [PATCH] i386 TLS_INIT_TP might produce bogus asm changing stack pointer.
- Authentication-results: sourceware.org; auth=none
- References: <1409041665-1558-1-git-send-email-mjw at redhat dot com> <53FC469A dot 2010701 at redhat dot com> <1409056041 dot 4970 dot 17 dot camel at bordewijk dot wildebeest dot org> <20140826212902 dot 05AD52C39C1 at topped-with-meat dot com>
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
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(-)
diff --git a/ChangeLog b/ChangeLog
index 6261ed4..ceeb06b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -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.
diff --git a/sysdeps/i386/nptl/tls.h b/sysdeps/i386/nptl/tls.h
index ac9c9a2..d7302ba 100644
--- a/sysdeps/i386/nptl/tls.h
+++ b/sysdeps/i386/nptl/tls.h
@@ -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