lkml.org 
[lkml]   [2018]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Subjectpowerpc syscall_set_return_value() is confused (was Re: [PATCH v6 18/27] powerpc: define syscall_get_error())
Date
Hi Dmitry,

"Dmitry V. Levin" <ldv@altlinux.org> writes:
> syscall_get_error() is required to be implemented on this
> architecture in addition to already implemented syscall_get_nr(),
> syscall_get_arguments(), syscall_get_return_value(), and
> syscall_get_arch() functions in order to extend the generic
> ptrace API with PTRACE_GET_SYSCALL_INFO request.
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Oleg Nesterov <oleg@redhat.com>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Elvira Khabirova <lineprinter@altlinux.org>
> Cc: Eugene Syromyatnikov <esyr@redhat.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Dmitry V. Levin <ldv@altlinux.org>
> ---
>
> Notes:
> v6: unchanged
>
> v5:
> This change has been tested with
> tools/testing/selftests/ptrace/get_syscall_info.c and strace,
> so it's correct from PTRACE_GET_SYSCALL_INFO point of view.
>
> This cast doubts on commit v4.3-rc1~86^2~81 that changed
> syscall_set_return_value() in a way that doesn't quite match
> syscall_get_error(), but syscall_set_return_value() is out
> of scope of this series, so I'll just let you know my concerns.

Sorry I only just saw this comment.

It's going to take me a while to page this stuff back into my brain, but
I think you may have a point.

I think the way it's written now *works* but only because it's only used
by seccomp, and we rely on the fact that the syscall exit path will
negate the value before returning to userspace or calling ptrace etc.

eg. we do:

syscall_set_return_value()
if (error) {
regs->ccr |= 0x10000000L;
regs->gpr[3] = error;

then the asm does:

/* Return code is already in r3 thanks to do_syscall_trace_enter() */
b .Lsyscall_exit
...

.Lsyscall_exit:
std r3,RESULT(r1)
...

3: cmpld r3,r11
ld r5,_CCR(r1)
bge- .Lsyscall_error
...

.Lsyscall_error:
oris r5,r5,0x1000 /* Set SO bit in CR */
neg r3,r3
std r5,_CCR(r1)

And we do the same before calling do_syscall_trace_leave().


Still it's a bit confused, because in the C code we're setting r3 and
CCR in the C code, but we're not negating the value in r3, and we're not
setting result at all.

I'll test a patch to fix it up.

cheers

\
 
 \ /
  Last update: 2018-12-18 13:37    [W:0.083 / U:0.256 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site