Patchwork [1/1] aarch64:fix aarch64_tlsdesc conflict handling

login
register
mail settings
Submitter Maninder Singh
Date Nov. 6, 2015, 6:59 a.m.
Message ID <1446793151-1563-1-git-send-email-maninder1.s@samsung.com>
Download mbox | patch
Permalink /patch/9576/
State New
Headers show

Comments

Maninder Singh - Nov. 6, 2015, 6:59 a.m.
This patch fixes aarch64_tlsdesc conflict handling in 
.gnu.conflict section. This patch is added to support prelink
on aarch64 architecture.
http://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit
/?h=cross_prelink_aarch64&id=2e83ded69171d87b76bcffaeffbd2cdb532663de

Signed-off-by: Vaneet Narang <v.narang@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
---
 sysdeps/aarch64/dl-machine.h |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)
Szabolcs Nagy - Nov. 6, 2015, 3:56 p.m.
On 06/11/15 06:59, Maninder Singh wrote:
> This patch fixes aarch64_tlsdesc conflict handling in
> .gnu.conflict section. This patch is added to support prelink
> on aarch64 architecture.
> http://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit
> /?h=cross_prelink_aarch64&id=2e83ded69171d87b76bcffaeffbd2cdb532663de
>
> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
> ---
>   sysdeps/aarch64/dl-machine.h |   17 ++++++++++++++---
>   1 files changed, 14 insertions(+), 3 deletions(-)
>
> diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
> index 217e179..70c112d 100644
> --- a/sysdeps/aarch64/dl-machine.h
> +++ b/sysdeps/aarch64/dl-machine.h
> @@ -290,9 +290,20 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>   #ifndef RTLD_BOOTSTRAP
>   	    if (! sym)
>   	      {
> -		td->arg = (void*)reloc->r_addend;
> -		td->entry = _dl_tlsdesc_undefweak;
> -	      }
> +			ElfW(Rela) * conflict_start = NULL;
> +			ElfW(Rela) * conflict_end = NULL;
> +			if ( l->l_info [ADDRIDX (DT_GNU_CONFLICT)])
> +		      {
> +				conflict_start = (ElfW(Rela) *) map->l_info [ADDRIDX (DT_GNU_CONFLICT)]->d_un.d_ptr;
> +				conflict_end = conflict_start +
> +								map->l_info [VALIDX (DT_GNU_CONFLICTSZ)]->d_un.d_val;
> +		      }
> +			td->arg = (void*)reloc->r_addend;
> +			if(conflict_start <= reloc && conflict_end >= reloc)
> +				td->entry = _dl_tlsdesc_return;
> +			else
> +				td->entry = _dl_tlsdesc_undefweak;

this code could be better organised.
(to have minimal impact on the non-prelinked case, e.g by
leaving the original code alone and moving all the new
special casing behind one if that checks for DT_GNU_CONFLICT.)

isn't it possible to just pass a dummy sym in
_dl_resolve_conflicts so we don't need any special
target specific code?

is x86_64 tlsdesc also broken with prelinking?

who uses prelinking and why? (it has various issues
including defeating security measures..)

> +	      }
>   	    else
>   #endif
>   	      {
>
Mark Hatle - Nov. 6, 2015, 4:09 p.m.
On 11/6/15 9:56 AM, Szabolcs Nagy wrote:
> On 06/11/15 06:59, Maninder Singh wrote:
>> This patch fixes aarch64_tlsdesc conflict handling in
>> .gnu.conflict section. This patch is added to support prelink
>> on aarch64 architecture.
>> http://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit
>> /?h=cross_prelink_aarch64&id=2e83ded69171d87b76bcffaeffbd2cdb532663de
>>
>> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>> ---
>>   sysdeps/aarch64/dl-machine.h |   17 ++++++++++++++---
>>   1 files changed, 14 insertions(+), 3 deletions(-)
>>
>> diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
>> index 217e179..70c112d 100644
>> --- a/sysdeps/aarch64/dl-machine.h
>> +++ b/sysdeps/aarch64/dl-machine.h
>> @@ -290,9 +290,20 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>>   #ifndef RTLD_BOOTSTRAP
>>   	    if (! sym)
>>   	      {
>> -		td->arg = (void*)reloc->r_addend;
>> -		td->entry = _dl_tlsdesc_undefweak;
>> -	      }
>> +			ElfW(Rela) * conflict_start = NULL;
>> +			ElfW(Rela) * conflict_end = NULL;
>> +			if ( l->l_info [ADDRIDX (DT_GNU_CONFLICT)])
>> +		      {
>> +				conflict_start = (ElfW(Rela) *) map->l_info [ADDRIDX (DT_GNU_CONFLICT)]->d_un.d_ptr;
>> +				conflict_end = conflict_start +
>> +								map->l_info [VALIDX (DT_GNU_CONFLICTSZ)]->d_un.d_val;
>> +		      }
>> +			td->arg = (void*)reloc->r_addend;
>> +			if(conflict_start <= reloc && conflict_end >= reloc)
>> +				td->entry = _dl_tlsdesc_return;
>> +			else
>> +				td->entry = _dl_tlsdesc_undefweak;
> 
> this code could be better organised.
> (to have minimal impact on the non-prelinked case, e.g by
> leaving the original code alone and moving all the new
> special casing behind one if that checks for DT_GNU_CONFLICT.)
> 
> isn't it possible to just pass a dummy sym in
> _dl_resolve_conflicts so we don't need any special
> target specific code?
> 
> is x86_64 tlsdesc also broken with prelinking?

As far as I am ware the tlsdesc on x86_64 is working properly.  The current set
of prelink tests pass.  I've not verified that that specific issue is tested for
however.  (Current tests are in the 'testsuite' directory and run via 'make
check'.  These tests recently exposed a problem between ld.so in glibc 2.22 and
the prelink interface BZ # 19178.  This interface problem affected all
i*86/x86_64 and arm/aarch64 systems.)

The current incarnation of the prelinker is available at
git://git.yoctoproject.org/prelink-cross

Where the 'master' branch is a replacement for the older prelink project (in
that it is standalone and needs to be compiled and run on the platform it is
targeting.)

While the 'cross_prelink' branch is the same code with the addition of an
emulated RTLD (prelink-rtld) as well as some code to handle sysroots.

(I've been maintaining an embedded oriented variant of the prelinker for some
time, but the upstream development seems to have stopped -- so I've attempted to
take over the core development as well.  If anyone is interested, see the git
repository or contact me for more information.)

> who uses prelinking and why? (it has various issues
> including defeating security measures..)

Anyone who has memory usage requirements (low memory available) or load time
performance requirements can benefit from the prelinker.

The prelinker does not 'defeat' security measures.  If your design needs to use
ASLR, then the prelinker is not for you.  (I won't debate the merits of ASLR,
other then to say it has tradeoffs which need to be decided on by the platform
integrator and it's not always the right answer.)

When prelinking a small embedded system, it's fairly typical to see 1/2 MB or
more in additional usable memory available after the system boot.  This is due
to fewer COW pages for the core system services such as sysvinit or systemd,
udev, etc.  In addition the load time of the components is reduced (as shown by
LD_DEBUG=statistics.)  These figures vary by system configuration (process
choice) and number of running services of course.

On a high end system with lots of memory and few load time speed requirements,
prelinker may not provide the benefits justifying it's use -- but on embedded
systems where performance and memory usage are often key software requirements
the prelinker can make the difference between a CPU core being adequate for the
task -- or a product doomed to failure.

>> +	      }
>>   	    else
>>   #endif
>>   	      {
>>
>
Szabolcs Nagy - Nov. 6, 2015, 5:05 p.m.
On 06/11/15 16:09, Mark Hatle wrote:
> On 11/6/15 9:56 AM, Szabolcs Nagy wrote:
>> On 06/11/15 06:59, Maninder Singh wrote:
>>> This patch fixes aarch64_tlsdesc conflict handling in
>>> .gnu.conflict section. This patch is added to support prelink
>>> on aarch64 architecture.
>>> http://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit
>>> /?h=cross_prelink_aarch64&id=2e83ded69171d87b76bcffaeffbd2cdb532663de
>>>
>>> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>>> ---
>>>    sysdeps/aarch64/dl-machine.h |   17 ++++++++++++++---
>>>    1 files changed, 14 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
>>> index 217e179..70c112d 100644
>>> --- a/sysdeps/aarch64/dl-machine.h
>>> +++ b/sysdeps/aarch64/dl-machine.h
>>> @@ -290,9 +290,20 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>>>    #ifndef RTLD_BOOTSTRAP
>>>    	    if (! sym)
>>>    	      {
>>> -		td->arg = (void*)reloc->r_addend;
>>> -		td->entry = _dl_tlsdesc_undefweak;
>>> -	      }
>>> +			ElfW(Rela) * conflict_start = NULL;
>>> +			ElfW(Rela) * conflict_end = NULL;
>>> +			if ( l->l_info [ADDRIDX (DT_GNU_CONFLICT)])
>>> +		      {
>>> +				conflict_start = (ElfW(Rela) *) map->l_info [ADDRIDX (DT_GNU_CONFLICT)]->d_un.d_ptr;
>>> +				conflict_end = conflict_start +
>>> +								map->l_info [VALIDX (DT_GNU_CONFLICTSZ)]->d_un.d_val;
>>> +		      }
>>> +			td->arg = (void*)reloc->r_addend;
>>> +			if(conflict_start <= reloc && conflict_end >= reloc)
>>> +				td->entry = _dl_tlsdesc_return;
>>> +			else
>>> +				td->entry = _dl_tlsdesc_undefweak;
>>
>> this code could be better organised.
>> (to have minimal impact on the non-prelinked case, e.g by
>> leaving the original code alone and moving all the new
>> special casing behind one if that checks for DT_GNU_CONFLICT.)
>>
>> isn't it possible to just pass a dummy sym in
>> _dl_resolve_conflicts so we don't need any special
>> target specific code?
>>
>> is x86_64 tlsdesc also broken with prelinking?
>
> As far as I am ware the tlsdesc on x86_64 is working properly.  The current set
> of prelink tests pass.  I've not verified that that specific issue is tested for
> however.  (Current tests are in the 'testsuite' directory and run via 'make
> check'.  These tests recently exposed a problem between ld.so in glibc 2.22 and
> the prelink interface BZ # 19178.  This interface problem affected all
> i*86/x86_64 and arm/aarch64 systems.)
>

i see failures with CC='gcc -mtls-dialect=gnu2'

11 of 43 tests failed
(3 tests were not run)

but i'm not sure if my setup is correct.

>> who uses prelinking and why? (it has various issues
>> including defeating security measures..)
>
> Anyone who has memory usage requirements (low memory available) or load time
> performance requirements can benefit from the prelinker.
>
> The prelinker does not 'defeat' security measures.  If your design needs to use
> ASLR, then the prelinker is not for you.  (I won't debate the merits of ASLR,
> other then to say it has tradeoffs which need to be decided on by the platform
> integrator and it's not always the right answer.)
>
> When prelinking a small embedded system, it's fairly typical to see 1/2 MB or
> more in additional usable memory available after the system boot.  This is due

you seem to care about memory usage but use glibc..

(i thought ppl in that domain use busybox, possibly static
linked, and don't go near glibc).

i personally still don't see the prelink hack justified,
but since glibc supports it, it should be fixed if something
is broken.

> to fewer COW pages for the core system services such as sysvinit or systemd,
> udev, etc.  In addition the load time of the components is reduced (as shown by
> LD_DEBUG=statistics.)  These figures vary by system configuration (process
> choice) and number of running services of course.
>
> On a high end system with lots of memory and few load time speed requirements,
> prelinker may not provide the benefits justifying it's use -- but on embedded
> systems where performance and memory usage are often key software requirements
> the prelinker can make the difference between a CPU core being adequate for the
> task -- or a product doomed to failure.
>
>>> +	      }
>>>    	    else
>>>    #endif
>>>    	      {
>>>
>>
>
Mark Hatle - Nov. 6, 2015, 5:44 p.m.
On 11/6/15 11:05 AM, Szabolcs Nagy wrote:
> On 06/11/15 16:09, Mark Hatle wrote:
>> On 11/6/15 9:56 AM, Szabolcs Nagy wrote:
>>> On 06/11/15 06:59, Maninder Singh wrote:
>>>> This patch fixes aarch64_tlsdesc conflict handling in
>>>> .gnu.conflict section. This patch is added to support prelink
>>>> on aarch64 architecture.
>>>> http://git.yoctoproject.org/cgit/cgit.cgi/prelink-cross/commit
>>>> /?h=cross_prelink_aarch64&id=2e83ded69171d87b76bcffaeffbd2cdb532663de
>>>>
>>>> Signed-off-by: Vaneet Narang <v.narang@samsung.com>
>>>> Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
>>>> ---
>>>>    sysdeps/aarch64/dl-machine.h |   17 ++++++++++++++---
>>>>    1 files changed, 14 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
>>>> index 217e179..70c112d 100644
>>>> --- a/sysdeps/aarch64/dl-machine.h
>>>> +++ b/sysdeps/aarch64/dl-machine.h
>>>> @@ -290,9 +290,20 @@ elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
>>>>    #ifndef RTLD_BOOTSTRAP
>>>>    	    if (! sym)
>>>>    	      {
>>>> -		td->arg = (void*)reloc->r_addend;
>>>> -		td->entry = _dl_tlsdesc_undefweak;
>>>> -	      }
>>>> +			ElfW(Rela) * conflict_start = NULL;
>>>> +			ElfW(Rela) * conflict_end = NULL;
>>>> +			if ( l->l_info [ADDRIDX (DT_GNU_CONFLICT)])
>>>> +		      {
>>>> +				conflict_start = (ElfW(Rela) *) map->l_info [ADDRIDX (DT_GNU_CONFLICT)]->d_un.d_ptr;
>>>> +				conflict_end = conflict_start +
>>>> +								map->l_info [VALIDX (DT_GNU_CONFLICTSZ)]->d_un.d_val;
>>>> +		      }
>>>> +			td->arg = (void*)reloc->r_addend;
>>>> +			if(conflict_start <= reloc && conflict_end >= reloc)
>>>> +				td->entry = _dl_tlsdesc_return;
>>>> +			else
>>>> +				td->entry = _dl_tlsdesc_undefweak;
>>>
>>> this code could be better organised.
>>> (to have minimal impact on the non-prelinked case, e.g by
>>> leaving the original code alone and moving all the new
>>> special casing behind one if that checks for DT_GNU_CONFLICT.)
>>>
>>> isn't it possible to just pass a dummy sym in
>>> _dl_resolve_conflicts so we don't need any special
>>> target specific code?
>>>
>>> is x86_64 tlsdesc also broken with prelinking?
>>
>> As far as I am ware the tlsdesc on x86_64 is working properly.  The current set
>> of prelink tests pass.  I've not verified that that specific issue is tested for
>> however.  (Current tests are in the 'testsuite' directory and run via 'make
>> check'.  These tests recently exposed a problem between ld.so in glibc 2.22 and
>> the prelink interface BZ # 19178.  This interface problem affected all
>> i*86/x86_64 and arm/aarch64 systems.)
>>
> 
> i see failures with CC='gcc -mtls-dialect=gnu2'
> 
> 11 of 43 tests failed
> (3 tests were not run)
> 
> but i'm not sure if my setup is correct.

My current configuration in based on the Yocto Project 2.0 (jethro) (about to be
released).

binutils - 2.25.1
gcc - 5.2.0 + patches
glibc - 2.22 + BZ 19178 fix (and a few others)

(I've also tested on some older hosts, like Fedora 21, Fedora 20, Ubuntu [not
sure], CentOS 5, and 6.)

In each case, we simply call 'gcc' directory without specific arguments.  (If
there are specific arguments that I should consider using for testings, I'm
certainly open to the suggestion.  I was unaware of the -mtls-dialect=gnu2 even
being a possibility for a production system at this point.  I thought it was
still experimental.)

2 of the tests are only run on 'check-harder' as they can run smaller machines
out of memory.

1 of the tests is simply a system setup test.

>>> who uses prelinking and why? (it has various issues
>>> including defeating security measures..)
>>
>> Anyone who has memory usage requirements (low memory available) or load time
>> performance requirements can benefit from the prelinker.
>>
>> The prelinker does not 'defeat' security measures.  If your design needs to use
>> ASLR, then the prelinker is not for you.  (I won't debate the merits of ASLR,
>> other then to say it has tradeoffs which need to be decided on by the platform
>> integrator and it's not always the right answer.)
>>
>> When prelinking a small embedded system, it's fairly typical to see 1/2 MB or
>> more in additional usable memory available after the system boot.  This is due
> 
> you seem to care about memory usage but use glibc..
> 
> (i thought ppl in that domain use busybox, possibly static
> linked, and don't go near glibc).

There is a large spectrum of devices in the Linux space.  Everything from "tiny"
systems where glibc is not used, and static binaries rule ... to small systems
where glibc is used and dynamic libraries are the norm .. to mid range that
start to resemble small/minimal server type designs .. to large systems that are
essentially special purpose servers.

glibc is the libc of choice for many of these systems because of the breadth of
software support -- and for support of tools like prelink.

> i personally still don't see the prelink hack justified,
> but since glibc supports it, it should be fixed if something
> is broken.

That is my goal.  Keep the prelinker working moving forward for the devices that
require it's functionality.  If this need does go away, then I may decide to
stop working on it.  But I believe we're a long way from that still.

--Mark

>> to fewer COW pages for the core system services such as sysvinit or systemd,
>> udev, etc.  In addition the load time of the components is reduced (as shown by
>> LD_DEBUG=statistics.)  These figures vary by system configuration (process
>> choice) and number of running services of course.
>>
>> On a high end system with lots of memory and few load time speed requirements,
>> prelinker may not provide the benefits justifying it's use -- but on embedded
>> systems where performance and memory usage are often key software requirements
>> the prelinker can make the difference between a CPU core being adequate for the
>> task -- or a product doomed to failure.
>>
>>>> +	      }
>>>>    	    else
>>>>    #endif
>>>>    	      {
>>>>
>>>
>>
>

Patch

diff --git a/sysdeps/aarch64/dl-machine.h b/sysdeps/aarch64/dl-machine.h
index 217e179..70c112d 100644
--- a/sysdeps/aarch64/dl-machine.h
+++ b/sysdeps/aarch64/dl-machine.h
@@ -290,9 +290,20 @@  elf_machine_rela (struct link_map *map, const ElfW(Rela) *reloc,
 #ifndef RTLD_BOOTSTRAP
 	    if (! sym)
 	      {
-		td->arg = (void*)reloc->r_addend;
-		td->entry = _dl_tlsdesc_undefweak;
-	      }
+			ElfW(Rela) * conflict_start = NULL;
+			ElfW(Rela) * conflict_end = NULL;
+			if ( l->l_info [ADDRIDX (DT_GNU_CONFLICT)])
+		      {
+				conflict_start = (ElfW(Rela) *) map->l_info [ADDRIDX (DT_GNU_CONFLICT)]->d_un.d_ptr;
+				conflict_end = conflict_start +
+								map->l_info [VALIDX (DT_GNU_CONFLICTSZ)]->d_un.d_val;
+		      }
+			td->arg = (void*)reloc->r_addend;
+			if(conflict_start <= reloc && conflict_end >= reloc)
+				td->entry = _dl_tlsdesc_return;
+			else
+				td->entry = _dl_tlsdesc_undefweak;
+	      }
 	    else
 #endif
 	      {