Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 25 Jun 2015 13:06:42 -0400
From: Rich Felker <dalias@...c.org>
To: Ted Hess <thess@...schensync.net>,
	OpenWrt developers <openwrt-devel@...ts.openwrt.org>,
	musl@...ts.openwall.com
Subject: Re: Re: [OpenWrt-Devel] Alsa-lib (libasound) segfaults on TLS
 variable (musl on mips)

On Thu, Jun 25, 2015 at 01:08:48AM +0200, Szabolcs Nagy wrote:
> * Szabolcs Nagy <nsz@...t70.net> [2015-06-24 22:57:54 +0200]:
> > the bug is that mips tls access uses a hard coded -32768
> > offset relative to whatever __tls_get_addr returned.
> > 
> > and musl did not account for this offset.
> > 
> 
> only affects mips shared objects with 'static __thread' variables,
> 
> extern __thread variables worked fine (and only those were
> tested in the libc-tests).
> 
> > the attached patch fixes the issue for me,
> > we will fix it in musl soon.
> 
> better patch attached that undoes the offset for extern tls
> vars in relocs handling.. (and with more consistent naming)

> diff --git a/arch/mips/pthread_arch.h b/arch/mips/pthread_arch.h
> index f8e35ae..904a248 100644
> --- a/arch/mips/pthread_arch.h
> +++ b/arch/mips/pthread_arch.h
> @@ -13,4 +13,6 @@ static inline struct pthread *__pthread_self()
>  #define TLS_ABOVE_TP
>  #define TP_ADJ(p) ((char *)(p) + sizeof(struct pthread) + 0x7000)
>  
> +#define DTP_OFFSET 0x8000
> +
>  #define CANCEL_REG_IP (3-(union {int __i; char __b;}){1}.__b)
> diff --git a/src/ldso/dynlink.c b/src/ldso/dynlink.c
> index b77c6f6..b4ca410 100644
> --- a/src/ldso/dynlink.c
> +++ b/src/ldso/dynlink.c
> @@ -337,7 +337,10 @@ static void do_relocs(struct dso *dso, size_t *rel, size_t rel_size, size_t stri
>  			*reloc_addr = def.dso->tls_id;
>  			break;
>  		case REL_DTPOFF:
> -			*reloc_addr = tls_val + addend;
> +#ifndef DTP_OFFSET
> +#define DTP_OFFSET 0
> +#endif
> +			*reloc_addr = tls_val + addend - DTP_OFFSET;
>  			break;
>  #ifdef TLS_ABOVE_TP
>  		case REL_TPOFF:
> diff --git a/src/thread/__tls_get_addr.c b/src/thread/__tls_get_addr.c
> index 3633396..94ea03c 100644
> --- a/src/thread/__tls_get_addr.c
> +++ b/src/thread/__tls_get_addr.c
> @@ -1,6 +1,10 @@
>  #include <stddef.h>
>  #include "pthread_impl.h"
>  
> +#ifndef DTP_OFFSET
> +#define DTP_OFFSET 0
> +#endif
> +
>  void *__tls_get_addr(size_t *v)
>  {
>  	pthread_t self = __pthread_self();
> @@ -8,9 +12,9 @@ void *__tls_get_addr(size_t *v)
>  	__attribute__((__visibility__("hidden")))
>  	void *__tls_get_new(size_t *);
>  	if (v[0]<=(size_t)self->dtv[0])
> -		return (char *)self->dtv[v[0]]+v[1];
> -	return __tls_get_new(v);
> +		return (char *)self->dtv[v[0]]+v[1]+DTP_OFFSET;
> +	return (char *)__tls_get_new(v)+DTP_OFFSET;
>  #else
> -	return (char *)self->dtv[1]+v[1];
> +	return (char *)self->dtv[1]+v[1]+DTP_OFFSET;
>  #endif
>  }

I think having additional instructions in __tls_get_addr is the wrong
approach. This function is the bottleneck for TLS access performance.
If the offset is needed, it should be stored in the dtv[i] pointers,
which then need to be adjusted at the point where they're setup.

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.