[04/21] sysdeps/{i386, x86_64}/mempcpy_chk.S: fix linknamespace for __mempcpy_chk

Message ID 20230620181910.1506893-5-fberat@redhat.com
State Committed
Commit 1bc85effd549ae42318b37555a4c76ebc479b92a
Headers
Series Allow glibc to be built with _FORTIFY_SOURCE |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Frederic Berat June 20, 2023, 6:18 p.m. UTC
  On i386 and x86_64, for libc.a specifically, __mempcpy_chk calls
mempcpy which leads POSIX routines to call non-POSIX mempcpy indirectly.

This leads the linknamespace test to fail when glibc is built with
__FORTIFY_SOURCE=3.

Since calling mempcpy doesn't bring any benefit for libc.a, directly
call __mempcpy instead.
---
 sysdeps/i386/mempcpy_chk.S   | 2 +-
 sysdeps/x86_64/mempcpy_chk.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Siddhesh Poyarekar June 21, 2023, 12:27 p.m. UTC | #1
On 2023-06-20 14:18, Frédéric Bérat wrote:
> On i386 and x86_64, for libc.a specifically, __mempcpy_chk calls
> mempcpy which leads POSIX routines to call non-POSIX mempcpy indirectly.
> 
> This leads the linknamespace test to fail when glibc is built with
> __FORTIFY_SOURCE=3.
> 
> Since calling mempcpy doesn't bring any benefit for libc.a, directly
> call __mempcpy instead.
> ---

LGTM as an independent fix.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

>   sysdeps/i386/mempcpy_chk.S   | 2 +-
>   sysdeps/x86_64/mempcpy_chk.S | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/i386/mempcpy_chk.S b/sysdeps/i386/mempcpy_chk.S
> index 8b785bd9a5..1e9bf71bfb 100644
> --- a/sysdeps/i386/mempcpy_chk.S
> +++ b/sysdeps/i386/mempcpy_chk.S
> @@ -28,6 +28,6 @@ ENTRY (__mempcpy_chk)
>   	movl	12(%esp), %eax
>   	cmpl	%eax, 16(%esp)
>   	jb	__chk_fail
> -	jmp	mempcpy
> +	jmp	__mempcpy
>   END (__mempcpy_chk)
>   #endif
> diff --git a/sysdeps/x86_64/mempcpy_chk.S b/sysdeps/x86_64/mempcpy_chk.S
> index b1ddb02f78..b60ee4ff08 100644
> --- a/sysdeps/x86_64/mempcpy_chk.S
> +++ b/sysdeps/x86_64/mempcpy_chk.S
> @@ -28,6 +28,6 @@
>   ENTRY (__mempcpy_chk)
>   	cmpq	%rdx, %rcx
>   	jb	__chk_fail
> -	jmp	mempcpy
> +	jmp	__mempcpy
>   END (__mempcpy_chk)
>   #endif
  
Noah Goldstein June 21, 2023, 5:26 p.m. UTC | #2
On 6/21/23, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
>
> On 2023-06-20 14:18, Frédéric Bérat wrote:
>> On i386 and x86_64, for libc.a specifically, __mempcpy_chk calls
>> mempcpy which leads POSIX routines to call non-POSIX mempcpy indirectly.
>>
>> This leads the linknamespace test to fail when glibc is built with
>> __FORTIFY_SOURCE=3.
>>
>> Since calling mempcpy doesn't bring any benefit for libc.a, directly
>> call __mempcpy instead.
>> ---
>
> LGTM as an independent fix.
>
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
>
>>   sysdeps/i386/mempcpy_chk.S   | 2 +-
>>   sysdeps/x86_64/mempcpy_chk.S | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/sysdeps/i386/mempcpy_chk.S b/sysdeps/i386/mempcpy_chk.S
>> index 8b785bd9a5..1e9bf71bfb 100644
>> --- a/sysdeps/i386/mempcpy_chk.S
>> +++ b/sysdeps/i386/mempcpy_chk.S
>> @@ -28,6 +28,6 @@ ENTRY (__mempcpy_chk)
>>   	movl	12(%esp), %eax
>>   	cmpl	%eax, 16(%esp)
>>   	jb	__chk_fail
>> -	jmp	mempcpy
>> +	jmp	__mempcpy
>>   END (__mempcpy_chk)
>>   #endif
>> diff --git a/sysdeps/x86_64/mempcpy_chk.S b/sysdeps/x86_64/mempcpy_chk.S
>> index b1ddb02f78..b60ee4ff08 100644
>> --- a/sysdeps/x86_64/mempcpy_chk.S
>> +++ b/sysdeps/x86_64/mempcpy_chk.S
>> @@ -28,6 +28,6 @@
>>   ENTRY (__mempcpy_chk)
>>   	cmpq	%rdx, %rcx
>>   	jb	__chk_fail
>> -	jmp	mempcpy
>> +	jmp	__mempcpy

Does this get the right ifunc (or isa version isa version of built
with cpu-specific flags)?
>>   END (__mempcpy_chk)
>>   #endif
>
  
Siddhesh Poyarekar June 22, 2023, 4:02 a.m. UTC | #3
On 2023-06-21 13:26, Noah Goldstein wrote:
>>> diff --git a/sysdeps/x86_64/mempcpy_chk.S b/sysdeps/x86_64/mempcpy_chk.S
>>> index b1ddb02f78..b60ee4ff08 100644
>>> --- a/sysdeps/x86_64/mempcpy_chk.S
>>> +++ b/sysdeps/x86_64/mempcpy_chk.S
>>> @@ -28,6 +28,6 @@
>>>    ENTRY (__mempcpy_chk)
>>>    	cmpq	%rdx, %rcx
>>>    	jb	__chk_fail
>>> -	jmp	mempcpy
>>> +	jmp	__mempcpy
> 
> Does this get the right ifunc (or isa version isa version of built
> with cpu-specific flags)?

It should; __mempcpy is basically an internal alias for mempcpy, which 
is an ifunc:

$ nm /lib64/libc.a | grep "i .*mempcpy"
0000000000000000 i __mempcpy
0000000000000000 i mempcpy

Sid
  

Patch

diff --git a/sysdeps/i386/mempcpy_chk.S b/sysdeps/i386/mempcpy_chk.S
index 8b785bd9a5..1e9bf71bfb 100644
--- a/sysdeps/i386/mempcpy_chk.S
+++ b/sysdeps/i386/mempcpy_chk.S
@@ -28,6 +28,6 @@  ENTRY (__mempcpy_chk)
 	movl	12(%esp), %eax
 	cmpl	%eax, 16(%esp)
 	jb	__chk_fail
-	jmp	mempcpy
+	jmp	__mempcpy
 END (__mempcpy_chk)
 #endif
diff --git a/sysdeps/x86_64/mempcpy_chk.S b/sysdeps/x86_64/mempcpy_chk.S
index b1ddb02f78..b60ee4ff08 100644
--- a/sysdeps/x86_64/mempcpy_chk.S
+++ b/sysdeps/x86_64/mempcpy_chk.S
@@ -28,6 +28,6 @@ 
 ENTRY (__mempcpy_chk)
 	cmpq	%rdx, %rcx
 	jb	__chk_fail
-	jmp	mempcpy
+	jmp	__mempcpy
 END (__mempcpy_chk)
 #endif