klibc - Sep 2014 - [PATCH] ppc64: ELFv2: Load TOC value in system call stub

This fixes a segmentation fault in the system call's error handling path
with
dynamically-linked binaries on PowerPC64 little endian.  The system call stub
wasn't loading up r2 with the appropriate TOC value in its global entry
point.

The r2 setup code comes from the FUNC_START macro in gcc [1] and an equivalent
one can also be found in the LOCALENTRY macro in glibc [2].

On the ELFv2 ABI (see [1]):
 - The global entry point is expected to load up r2 with the appropriate TOC
   value for this function.
 - The local entry point expects r2 to be set up to the current TOC.

The problem happened with dynamically-linked binaries because:
 - the system call is an indirect call (via global entry point) from the binary
   to the shared library, landing in the syscall stub  (which didn't load up
r2
   with the TOC of the shared library)
 - its branch to __syscall_error is a direct call (via local entry point) within
   the shared library, landing in the function (which expects r2 to be set up to
   that TOC)
 - when the function attempts to store errno (in an address relative to the
TOC),
   that address incorrectly pointed to a read-only segment -- segmentation
fault.

The problem didn't happen with statically-linked binaries because the TOC
value
wasn't different on that case.

Thanks and credits to Alan Modra and Ulrich Weigand, for helping with this and
pointing out the solution.

[1] https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01141.html
[2] https://www.sourceware.org/ml/libc-alpha/2013-11/msg00315.html

Signed-off-by: Mauricio Faria de Oliveira <mauricfo at linux.vnet.ibm.com>
---
 usr/klibc/arch/ppc64/sysstub.ph |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/usr/klibc/arch/ppc64/sysstub.ph b/usr/klibc/arch/ppc64/sysstub.ph
index b3f6e38..a0c6d41 100644
--- a/usr/klibc/arch/ppc64/sysstub.ph
+++ b/usr/klibc/arch/ppc64/sysstub.ph
@@ -18,6 +18,9 @@ sub make_sysstub($$$$$@) {
 #if _CALL_ELF == 2
 	.type ${fname},\@function
 ${fname}:
+0:	addis	2,12,(.TOC.-0b)\@ha
+	addi	2,2,(.TOC.-0b)\@l
+	.localentry ${fname},.-${fname}
 #else
 	.section ".opd","aw"
 	.balign 8
-- 
1.7.1

On 09/09/2014 07:17 PM, Mauricio Faria de Oliveira
wrote:
> This fixes a segmentation fault in the system call's error handling
path with
> dynamically-linked binaries on PowerPC64 little endian.  The system call
stub
> wasn't loading up r2 with the appropriate TOC value in its global entry
point.
Test results on Debian ppc64el. Probably too detailed, but documented.

Before:
-------

Test-case:

	'stat.shared' segfaults with a non-existing file

	$ rm -f no-file

	$ usr/klibc/tests/stat no-file
	no-file: No such file or directory

	$ usr/klibc/tests/stat.shared no-file
	Segmentation fault

Test-suite:

	Comparing statically- vs. dinamically-linked binaries.
	One difference: fcntl's return code.

	$ find usr/klibc/tests/ -type f -executable -not -name '*.g' -not -name
'*.shared' \
	    | while read test; do $test >/dev/null 2>&1; rc=$?; [ $rc -ne 0 ]
&& echo "failed: $test (rc: $rc)"; done | sort
	failed: usr/klibc/tests/fcntl (rc: 1)
	failed: usr/klibc/tests/fnmatch (rc: 139)
	failed: usr/klibc/tests/testrand48 (rc: 1)

	$ find usr/klibc/tests/ -type f -executable -not -name '*.g'  -name 
'*.shared' \
	    | while read test; do $test >/dev/null 2>&1; rc=$?; [ $rc -ne 0 ]
&& echo "failed: $test (rc: $rc)"; done | sort
	failed: usr/klibc/tests/fcntl.shared (rc: 139)
	failed: usr/klibc/tests/fnmatch.shared (rc: 139)
	failed: usr/klibc/tests/testrand48.shared (rc: 1)

Generated instructions:

	No r2 setup before branch to local entry point (+8 offset).

	(gdb) disass
	Dump of assembler code for function stat:
	=> 0x000000000f007854 <+0>:     li      r0,106
	   0x000000000f007858 <+4>:     sc
	   0x000000000f00785c <+8>:     bnslr
	   0x000000000f007860 <+12>:    b       0xf012cb4
<__syscall_error+8>
	End of assembler dump.

After:
-----

Test-case:

	'stat.shared' no longer segfaults with a non-existing file
	
	$ rm -f no-file

	$ usr/klibc/tests/stat no-file
	no-file: No such file or directory

	$ usr/klibc/tests/stat.shared no-file
	no-file: No such file or directory

Test-suite:

	No regressions introduced.
	No differences between statically- vs. dynamically-linked binaries.

	$ find usr/klibc/tests/ -type f -executable -not -name '*.g' -not -name
'*.shared' | while read test; do $test >/dev/null 2>&1; rc=$?;
[ $rc -ne
0 ] && echo "failed: $test (rc: $rc)"; done | sort
	failed: usr/klibc/tests/fcntl (rc: 1)
	failed: usr/klibc/tests/fnmatch (rc: 139)
	failed: usr/klibc/tests/testrand48 (rc: 1)

	$ find usr/klibc/tests/ -type f -executable -not -name '*.g'  -name 
'*.shared' | while read test; do $test >/dev/null 2>&1; rc=$?;
[ $rc -ne
0 ] && echo "failed: $test (rc: $rc)"; done | sort
	failed: usr/klibc/tests/fcntl.shared (rc: 1)
	failed: usr/klibc/tests/fnmatch.shared (rc: 139)
	failed: usr/klibc/tests/testrand48.shared (rc: 1)


Generated instructions

	Now r2 _is_ setup before branch to local entry point (+8 offset).

	(gdb) disass
	Dump of assembler code for function stat:
	=> 0x000000000f007a84 <+0>:     lis     r2,3843
	   0x000000000f007a88 <+4>:     addi    r2,r2,-3816
	   0x000000000f007a8c <+8>:     li      r0,106
	   0x000000000f007a90 <+12>:    sc
	   0x000000000f007a94 <+16>:    bnslr
	   0x000000000f007a98 <+20>:    b       0xf0131cc
<__syscall_error+8>
	End of assembler dump.





-- 
Mauricio Faria de Oliveira
IBM Linux Technology Center

On Tue, Sep 09, 2014 at 07:17:19PM -0300, Mauricio Faria de Oliveira
wrote:
>  usr/klibc/arch/ppc64/sysstub.ph |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/usr/klibc/arch/ppc64/sysstub.ph
b/usr/klibc/arch/ppc64/sysstub.ph
> index b3f6e38..a0c6d41 100644
> --- a/usr/klibc/arch/ppc64/sysstub.ph
> +++ b/usr/klibc/arch/ppc64/sysstub.ph
> @@ -18,6 +18,9 @@ sub make_sysstub($$$$$@) {
>  #if _CALL_ELF == 2
>  	.type ${fname},\@function
>  ${fname}:
> +0:	addis	2,12,(.TOC.-0b)\@ha
> +	addi	2,2,(.TOC.-0b)\@l
> +	.localentry ${fname},.-${fname}
>  #else
>  	.section ".opd","aw"
>  	.balign 8
> -- 
> 1.7.1
Looks good.

-- 
Alan Modra
Australia Development Lab, IBM

On Tue, Sep 09, 2014 at 07:17:19PM -0300, Mauricio Faria de Oliveira
wrote:
> This fixes a segmentation fault in the system call's error handling
path with
> dynamically-linked binaries on PowerPC64 little endian.  The system call
stub
> wasn't loading up r2 with the appropriate TOC value in its global entry
point.
> 
> The r2 setup code comes from the FUNC_START macro in gcc [1] and an
equivalent
> one can also be found in the LOCALENTRY macro in glibc [2].
> 
> On the ELFv2 ABI (see [1]):
>  - The global entry point is expected to load up r2 with the appropriate
TOC
>    value for this function.
>  - The local entry point expects r2 to be set up to the current TOC.
> 
> The problem happened with dynamically-linked binaries because:
>  - the system call is an indirect call (via global entry point) from the
binary
>    to the shared library, landing in the syscall stub  (which didn't
load up r2
>    with the TOC of the shared library)
>  - its branch to __syscall_error is a direct call (via local entry point)
within
>    the shared library, landing in the function (which expects r2 to be set
up to
>    that TOC)
>  - when the function attempts to store errno (in an address relative to the
TOC),
>    that address incorrectly pointed to a read-only segment -- segmentation
fault.
> 
> The problem didn't happen with statically-linked binaries because the
TOC value
> wasn't different on that case.
> 
> Thanks and credits to Alan Modra and Ulrich Weigand, for helping with this
and
> pointing out the solution.
> 
> [1] https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01141.html
> [2] https://www.sourceware.org/ml/libc-alpha/2013-11/msg00315.html
> 
> Signed-off-by: Mauricio Faria de Oliveira <mauricfo at
linux.vnet.ibm.com>
> ---
>  usr/klibc/arch/ppc64/sysstub.ph |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/usr/klibc/arch/ppc64/sysstub.ph
b/usr/klibc/arch/ppc64/sysstub.ph
> index b3f6e38..a0c6d41 100644
> --- a/usr/klibc/arch/ppc64/sysstub.ph
> +++ b/usr/klibc/arch/ppc64/sysstub.ph
> @@ -18,6 +18,9 @@ sub make_sysstub($$$$$@) {
>  #if _CALL_ELF == 2
>  	.type ${fname},\@function
>  ${fname}:
> +0:	addis	2,12,(.TOC.-0b)\@ha
> +	addi	2,2,(.TOC.-0b)\@l
> +	.localentry ${fname},.-${fname}
>  #else
>  	.section ".opd","aw"
>  	.balign 8
Thanks for the patch, I confirm it fixes the problem.

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien at aurel32.net                 http://www.aurel32.net