[Devel,RH7] ms/x86/signal: Remove bogus user_64bit_mode() check from sigaction_compat_abi()

Submitted by Dmitry Safonov on Oct. 26, 2016, 4:15 p.m.

Details

Message ID 20161026161527.1352-2-dsafonov@virtuozzo.com
State New
Series "ms/x86/signal: Remove bogus user_64bit_mode() check from sigaction_compat_abi()"
Headers show

Commit Message

Dmitry Safonov Oct. 26, 2016, 4:15 p.m.
The recent introduction of SA_X32/IA32 sa_flags added a check for
user_64bit_mode() into sigaction_compat_abi(). user_64bit_mode() is true
for native 64-bit processes and x32 processes.

Due to that the function returns w/o setting the SA_X32_ABI flag for X32
processes. In consequence the kernel attempts to deliver the signal to the
X32 process in native 64-bit mode causing the process to segfault.

Remove the check, so the actual check for X32 mode which sets the ABI flag
can be reached. There is no side effect for native 64-bit mode.

[ tglx: Rewrote changelog ]

Fixes: 6846351052e6 ("x86/signal: Add SA_{X32,IA32}_ABI sa_flags")
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Tested-by: Adam Borowski <kilobyte@angband.pl>
Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com>
Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: linux-mm@kvack.org
Cc: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Pavel Emelyanov <xemul@virtuozzo.com>
Link: http://lkml.kernel.org/r/CAJwJo6Z8ZWPqNfT6t-i8GW1MKxQrKDUagQqnZ%2B0%2B697%3DMyVeGg@mail.gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>

[backported from ms commit ed1e7db33c33 ("x86/signal: Remove bogus
user_64bit_mode() check from sigaction_compat_abi()")]
Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
---
 arch/x86/ia32/ia32_signal.c | 3 ---
 1 file changed, 3 deletions(-)

Patch hide | download patch | download mbox

diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
index 9c6c4de8da46..cbd5c5ad5edf 100644
--- a/arch/x86/ia32/ia32_signal.c
+++ b/arch/x86/ia32/ia32_signal.c
@@ -47,9 +47,6 @@  void sigaction_compat_abi(struct k_sigaction *act, struct k_sigaction *oact)
 	/* Don't let flags to be set from userspace */
 	act->sa.sa_flags &= ~(SA_IA32_ABI | SA_X32_ABI);
 
-	if (user_64bit_mode(current_pt_regs()))
-		return;
-
 	if (is_ia32_task())
 		act->sa.sa_flags |= SA_IA32_ABI;
 	if (is_x32_task())

Comments

Andrey Ryabinin Oct. 27, 2016, 3 p.m.
On 10/26/2016 07:15 PM, Dmitry Safonov wrote:
> The recent introduction of SA_X32/IA32 sa_flags added a check for
> user_64bit_mode() into sigaction_compat_abi(). user_64bit_mode() is true
> for native 64-bit processes and x32 processes.
> 
> Due to that the function returns w/o setting the SA_X32_ABI flag for X32
> processes. In consequence the kernel attempts to deliver the signal to the
> X32 process in native 64-bit mode causing the process to segfault.
> 
> Remove the check, so the actual check for X32 mode which sets the ABI flag
> can be reached. There is no side effect for native 64-bit mode.
> 
> [ tglx: Rewrote changelog ]
> 
> Fixes: 6846351052e6 ("x86/signal: Add SA_{X32,IA32}_ABI sa_flags")
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Tested-by: Adam Borowski <kilobyte@angband.pl>
> Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com>
> Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: linux-mm@kvack.org
> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
> Link: http://lkml.kernel.org/r/CAJwJo6Z8ZWPqNfT6t-i8GW1MKxQrKDUagQqnZ%2B0%2B697%3DMyVeGg@mail.gmail.com
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> 
> [backported from ms commit ed1e7db33c33 ("x86/signal: Remove bogus
> user_64bit_mode() check from sigaction_compat_abi()")]
> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
> ---

Acked-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

>  arch/x86/ia32/ia32_signal.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
> index 9c6c4de8da46..cbd5c5ad5edf 100644
> --- a/arch/x86/ia32/ia32_signal.c
> +++ b/arch/x86/ia32/ia32_signal.c
> @@ -47,9 +47,6 @@ void sigaction_compat_abi(struct k_sigaction *act, struct k_sigaction *oact)
>  	/* Don't let flags to be set from userspace */
>  	act->sa.sa_flags &= ~(SA_IA32_ABI | SA_X32_ABI);
>  
> -	if (user_64bit_mode(current_pt_regs()))
> -		return;
> -
>  	if (is_ia32_task())
>  		act->sa.sa_flags |= SA_IA32_ABI;
>  	if (is_x32_task())
>
Dmitry Safonov Nov. 23, 2016, 6:36 p.m.
On 10/27/2016 06:00 PM, Andrey Ryabinin wrote:
> On 10/26/2016 07:15 PM, Dmitry Safonov wrote:
>> The recent introduction of SA_X32/IA32 sa_flags added a check for
>> user_64bit_mode() into sigaction_compat_abi(). user_64bit_mode() is true
>> for native 64-bit processes and x32 processes.
>>
>> Due to that the function returns w/o setting the SA_X32_ABI flag for X32
>> processes. In consequence the kernel attempts to deliver the signal to the
>> X32 process in native 64-bit mode causing the process to segfault.
>>
>> Remove the check, so the actual check for X32 mode which sets the ABI flag
>> can be reached. There is no side effect for native 64-bit mode.
>>
>> [ tglx: Rewrote changelog ]
>>
>> Fixes: 6846351052e6 ("x86/signal: Add SA_{X32,IA32}_ABI sa_flags")
>> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
>> Tested-by: Adam Borowski <kilobyte@angband.pl>
>> Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com>
>> Cc: Dmitry Safonov <dsafonov@virtuozzo.com>
>> Cc: Oleg Nesterov <oleg@redhat.com>
>> Cc: linux-mm@kvack.org
>> Cc: Cyrill Gorcunov <gorcunov@openvz.org>
>> Cc: Pavel Emelyanov <xemul@virtuozzo.com>
>> Link: http://lkml.kernel.org/r/CAJwJo6Z8ZWPqNfT6t-i8GW1MKxQrKDUagQqnZ%2B0%2B697%3DMyVeGg@mail.gmail.com
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> Signed-off-by: Ingo Molnar <mingo@kernel.org>
>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>>
>> [backported from ms commit ed1e7db33c33 ("x86/signal: Remove bogus
>> user_64bit_mode() check from sigaction_compat_abi()")]
>> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com>
>> ---
>
> Acked-by: Andrey Ryabinin <aryabinin@virtuozzo.com>

ping? Can we have it?
It's quite rare to use x32 (not ia32 compat), quite sure that we don't
even have x32 libc and other userspace in repos, but still...

>>  arch/x86/ia32/ia32_signal.c | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/arch/x86/ia32/ia32_signal.c b/arch/x86/ia32/ia32_signal.c
>> index 9c6c4de8da46..cbd5c5ad5edf 100644
>> --- a/arch/x86/ia32/ia32_signal.c
>> +++ b/arch/x86/ia32/ia32_signal.c
>> @@ -47,9 +47,6 @@ void sigaction_compat_abi(struct k_sigaction *act, struct k_sigaction *oact)
>>  	/* Don't let flags to be set from userspace */
>>  	act->sa.sa_flags &= ~(SA_IA32_ABI | SA_X32_ABI);
>>
>> -	if (user_64bit_mode(current_pt_regs()))
>> -		return;
>> -
>>  	if (is_ia32_task())
>>  		act->sa.sa_flags |= SA_IA32_ABI;
>>  	if (is_x32_task())
>>