diff mbox

[v3,1/3] powerpc: Don't force ENOSYS as error on syscall fail

Message ID 1423815752-3013-2-git-send-email-bogdan.purcareata@freescale.com (mailing list archive)
State Superseded
Delegated to: Michael Ellerman
Headers show

Commit Message

Bogdan Purcareata Feb. 13, 2015, 8:22 a.m. UTC
In certain scenarios - e.g. seccomp filtering with ERRNO as default action -
the system call fails for other reasons than the syscall not being available.
The seccomp filter can be configured to store a user-defined error code on
return from a blacklisted syscall. Don't always set ENOSYS on
do_syscall_trace_enter failure.

Delegate setting ENOSYS in case of failure, where appropriate, to
do_syscall_trace_enter.

v3:
- keep setting ENOSYS in the syscall entry assembly for scenarios without
  syscall tracing

v2:
- move setting ENOSYS as errno from the syscall entry assembly to
  do_syscall_trace_enter, only in the specific case

Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com>
---
 arch/powerpc/kernel/entry_32.S | 7 ++++++-
 arch/powerpc/kernel/entry_64.S | 5 +++--
 arch/powerpc/kernel/ptrace.c   | 4 +++-
 3 files changed, 12 insertions(+), 4 deletions(-)

Comments

Mike Strosaker Feb. 18, 2015, 3:01 a.m. UTC | #1
This patch failed to build using pseries_le_defconfig.  With the change 
noted below in entry_64.S, the build succeeded and seccomp mode 2 worked 
correctly.

Best,
Mike Strosaker

On 02/13/2015 02:22 AM, Bogdan Purcareata wrote:
> In certain scenarios - e.g. seccomp filtering with ERRNO as default action -
> the system call fails for other reasons than the syscall not being available.
> The seccomp filter can be configured to store a user-defined error code on
> return from a blacklisted syscall. Don't always set ENOSYS on
> do_syscall_trace_enter failure.
>
> Delegate setting ENOSYS in case of failure, where appropriate, to
> do_syscall_trace_enter.
>
> v3:
> - keep setting ENOSYS in the syscall entry assembly for scenarios without
>    syscall tracing
>
> v2:
> - move setting ENOSYS as errno from the syscall entry assembly to
>    do_syscall_trace_enter, only in the specific case
>
> Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com>
> ---
>   arch/powerpc/kernel/entry_32.S | 7 ++++++-
>   arch/powerpc/kernel/entry_64.S | 5 +++--
>   arch/powerpc/kernel/ptrace.c   | 4 +++-
>   3 files changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 46fc0f4..b2f88cd 100644
> --- a/arch/powerpc/kernel/entry_32.S
> +++ b/arch/powerpc/kernel/entry_32.S
> @@ -333,12 +333,12 @@ _GLOBAL(DoSyscall)
>   	lwz	r11,TI_FLAGS(r10)
>   	andi.	r11,r11,_TIF_SYSCALL_DOTRACE
>   	bne-	syscall_dotrace
> -syscall_dotrace_cont:
>   	cmplwi	0,r0,NR_syscalls
>   	lis	r10,sys_call_table@h
>   	ori	r10,r10,sys_call_table@l
>   	slwi	r0,r0,2
>   	bge-	66f
> +syscall_dotrace_cont:
>   	lwzx	r10,r10,r0	/* Fetch system call handler [ptr] */
>   	mtlr	r10
>   	addi	r9,r1,STACK_FRAME_OVERHEAD
> @@ -457,6 +457,11 @@ syscall_dotrace:
>   	lwz	r7,GPR7(r1)
>   	lwz	r8,GPR8(r1)
>   	REST_NVGPRS(r1)
> +	cmplwi	0,r0,NR_syscalls
> +	lis	r10,sys_call_table@h
> +	ori	r10,r10,sys_call_table@l
> +	slwi	r0,r0,2
> +	bge-	ret_from_syscall
>   	b	syscall_dotrace_cont
>
>   syscall_exit_work:
> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
> index d180caf2..0d22fa8 100644
> --- a/arch/powerpc/kernel/entry_64.S
> +++ b/arch/powerpc/kernel/entry_64.S
> @@ -144,7 +144,6 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
>   	ld	r10,TI_FLAGS(r11)
>   	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
>   	bne	syscall_dotrace
> -.Lsyscall_dotrace_cont:
>   	cmpldi	0,r0,NR_syscalls
>   	bge-	syscall_enosys
>
> @@ -253,7 +252,9 @@ syscall_dotrace:
>   	addi	r9,r1,STACK_FRAME_OVERHEAD
>   	CURRENT_THREAD_INFO(r10, r1)
>   	ld	r10,TI_FLAGS(r10)
> -	b	.Lsyscall_dotrace_cont
> +	cmpldi	0,r0,NR_syscalls
> +	bge-	syscall_exit

Shouldn't this be .Lsyscall_exit?

> +	b	system_call
>
>   syscall_enosys:
>   	li	r3,-ENOSYS
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index f21897b..2edae06 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -1775,13 +1775,15 @@ long do_syscall_trace_enter(struct pt_regs *regs)
>   	secure_computing_strict(regs->gpr[0]);
>
>   	if (test_thread_flag(TIF_SYSCALL_TRACE) &&
> -	    tracehook_report_syscall_entry(regs))
> +	    tracehook_report_syscall_entry(regs)) {
>   		/*
>   		 * Tracing decided this syscall should not happen.
>   		 * We'll return a bogus call number to get an ENOSYS
>   		 * error, but leave the original number in regs->gpr[0].
>   		 */
>   		ret = -1L;
> +		syscall_set_return_value(current, regs, ENOSYS, 0);
> +	}
>
>   	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
>   		trace_sys_enter(regs, regs->gpr[0]);
>
Purcareata Bogdan Feb. 18, 2015, 6:50 a.m. UTC | #2
On 18.02.2015 05:01, Mike Strosaker wrote:
> This patch failed to build using pseries_le_defconfig.  With the change
> noted below in entry_64.S, the build succeeded and seccomp mode 2 worked
> correctly.
>
> Best,
> Mike Strosaker
>
> On 02/13/2015 02:22 AM, Bogdan Purcareata wrote:
>> In certain scenarios - e.g. seccomp filtering with ERRNO as default action -
>> the system call fails for other reasons than the syscall not being available.
>> The seccomp filter can be configured to store a user-defined error code on
>> return from a blacklisted syscall. Don't always set ENOSYS on
>> do_syscall_trace_enter failure.
>>
>> Delegate setting ENOSYS in case of failure, where appropriate, to
>> do_syscall_trace_enter.
>>
>> v3:
>> - keep setting ENOSYS in the syscall entry assembly for scenarios without
>>     syscall tracing
>>
>> v2:
>> - move setting ENOSYS as errno from the syscall entry assembly to
>>     do_syscall_trace_enter, only in the specific case
>>
>> Signed-off-by: Bogdan Purcareata <bogdan.purcareata@freescale.com>
>> ---
>>    arch/powerpc/kernel/entry_32.S | 7 ++++++-
>>    arch/powerpc/kernel/entry_64.S | 5 +++--
>>    arch/powerpc/kernel/ptrace.c   | 4 +++-
>>    3 files changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 46fc0f4..b2f88cd 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -333,12 +333,12 @@ _GLOBAL(DoSyscall)
>>    	lwz	r11,TI_FLAGS(r10)
>>    	andi.	r11,r11,_TIF_SYSCALL_DOTRACE
>>    	bne-	syscall_dotrace
>> -syscall_dotrace_cont:
>>    	cmplwi	0,r0,NR_syscalls
>>    	lis	r10,sys_call_table@h
>>    	ori	r10,r10,sys_call_table@l
>>    	slwi	r0,r0,2
>>    	bge-	66f
>> +syscall_dotrace_cont:
>>    	lwzx	r10,r10,r0	/* Fetch system call handler [ptr] */
>>    	mtlr	r10
>>    	addi	r9,r1,STACK_FRAME_OVERHEAD
>> @@ -457,6 +457,11 @@ syscall_dotrace:
>>    	lwz	r7,GPR7(r1)
>>    	lwz	r8,GPR8(r1)
>>    	REST_NVGPRS(r1)
>> +	cmplwi	0,r0,NR_syscalls
>> +	lis	r10,sys_call_table@h
>> +	ori	r10,r10,sys_call_table@l
>> +	slwi	r0,r0,2
>> +	bge-	ret_from_syscall
>>    	b	syscall_dotrace_cont
>>
>>    syscall_exit_work:
>> diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
>> index d180caf2..0d22fa8 100644
>> --- a/arch/powerpc/kernel/entry_64.S
>> +++ b/arch/powerpc/kernel/entry_64.S
>> @@ -144,7 +144,6 @@ END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
>>    	ld	r10,TI_FLAGS(r11)
>>    	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
>>    	bne	syscall_dotrace
>> -.Lsyscall_dotrace_cont:
>>    	cmpldi	0,r0,NR_syscalls
>>    	bge-	syscall_enosys
>>
>> @@ -253,7 +252,9 @@ syscall_dotrace:
>>    	addi	r9,r1,STACK_FRAME_OVERHEAD
>>    	CURRENT_THREAD_INFO(r10, r1)
>>    	ld	r10,TI_FLAGS(r10)
>> -	b	.Lsyscall_dotrace_cont
>> +	cmpldi	0,r0,NR_syscalls
>> +	bge-	syscall_exit
>
> Shouldn't this be .Lsyscall_exit?

Thanks for testing and spotting this! The kernel I tested with didn't 
have syscall_exit converted to a local label (commit 
4c3b21686111e0ac6018469dacbc5549f9915cf8).

Will resend with this change.

Bogdan P.

>> +	b	system_call
>>
>>    syscall_enosys:
>>    	li	r3,-ENOSYS
>> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
>> index f21897b..2edae06 100644
>> --- a/arch/powerpc/kernel/ptrace.c
>> +++ b/arch/powerpc/kernel/ptrace.c
>> @@ -1775,13 +1775,15 @@ long do_syscall_trace_enter(struct pt_regs *regs)
>>    	secure_computing_strict(regs->gpr[0]);
>>
>>    	if (test_thread_flag(TIF_SYSCALL_TRACE) &&
>> -	    tracehook_report_syscall_entry(regs))
>> +	    tracehook_report_syscall_entry(regs)) {
>>    		/*
>>    		 * Tracing decided this syscall should not happen.
>>    		 * We'll return a bogus call number to get an ENOSYS
>>    		 * error, but leave the original number in regs->gpr[0].
>>    		 */
>>    		ret = -1L;
>> +		syscall_set_return_value(current, regs, ENOSYS, 0);
>> +	}
>>
>>    	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
>>    		trace_sys_enter(regs, regs->gpr[0]);
>>
>
diff mbox

Patch

diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 46fc0f4..b2f88cd 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -333,12 +333,12 @@  _GLOBAL(DoSyscall)
 	lwz	r11,TI_FLAGS(r10)
 	andi.	r11,r11,_TIF_SYSCALL_DOTRACE
 	bne-	syscall_dotrace
-syscall_dotrace_cont:
 	cmplwi	0,r0,NR_syscalls
 	lis	r10,sys_call_table@h
 	ori	r10,r10,sys_call_table@l
 	slwi	r0,r0,2
 	bge-	66f
+syscall_dotrace_cont:
 	lwzx	r10,r10,r0	/* Fetch system call handler [ptr] */
 	mtlr	r10
 	addi	r9,r1,STACK_FRAME_OVERHEAD
@@ -457,6 +457,11 @@  syscall_dotrace:
 	lwz	r7,GPR7(r1)
 	lwz	r8,GPR8(r1)
 	REST_NVGPRS(r1)
+	cmplwi	0,r0,NR_syscalls
+	lis	r10,sys_call_table@h
+	ori	r10,r10,sys_call_table@l
+	slwi	r0,r0,2
+	bge-	ret_from_syscall
 	b	syscall_dotrace_cont
 
 syscall_exit_work:
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index d180caf2..0d22fa8 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -144,7 +144,6 @@  END_FW_FTR_SECTION_IFSET(FW_FEATURE_SPLPAR)
 	ld	r10,TI_FLAGS(r11)
 	andi.	r11,r10,_TIF_SYSCALL_DOTRACE
 	bne	syscall_dotrace
-.Lsyscall_dotrace_cont:
 	cmpldi	0,r0,NR_syscalls
 	bge-	syscall_enosys
 
@@ -253,7 +252,9 @@  syscall_dotrace:
 	addi	r9,r1,STACK_FRAME_OVERHEAD
 	CURRENT_THREAD_INFO(r10, r1)
 	ld	r10,TI_FLAGS(r10)
-	b	.Lsyscall_dotrace_cont
+	cmpldi	0,r0,NR_syscalls
+	bge-	syscall_exit
+	b	system_call
 
 syscall_enosys:
 	li	r3,-ENOSYS
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index f21897b..2edae06 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1775,13 +1775,15 @@  long do_syscall_trace_enter(struct pt_regs *regs)
 	secure_computing_strict(regs->gpr[0]);
 
 	if (test_thread_flag(TIF_SYSCALL_TRACE) &&
-	    tracehook_report_syscall_entry(regs))
+	    tracehook_report_syscall_entry(regs)) {
 		/*
 		 * Tracing decided this syscall should not happen.
 		 * We'll return a bogus call number to get an ENOSYS
 		 * error, but leave the original number in regs->gpr[0].
 		 */
 		ret = -1L;
+		syscall_set_return_value(current, regs, ENOSYS, 0);
+	}
 
 	if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
 		trace_sys_enter(regs, regs->gpr[0]);