* [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls
@ 2021-02-21 0:25 Sergei Trofimovich
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Sergei Trofimovich @ 2021-02-21 0:25 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Sergei Trofimovich, Oleg Nesterov, linux-ia64, Dmitry V . Levin
In https://bugs.gentoo.org/769614 Dmitry noticed that
`ptrace(PTRACE_GET_SYSCALL_INFO)` does not work for syscalls called
via glibc's syscall() wrapper.
ia64 has two ways to call syscalls from userspace: via `break` and via
`eps` instructions.
The difference is in stack layout:
1. `eps` creates simple stack frame: no locals, in{0..7} == out{0..8}
2. `break` uses userspace stack frame: may be locals (glibc provides
one), in{0..7} == out{0..8}.
Both work fine in syscall handling cde itself.
But `ptrace(PTRACE_GET_SYSCALL_INFO)` uses unwind mechanism to
re-extract syscall arguments but it does not account for locals.
The change always skips locals registers. It should not change `eps`
path as kernel's handler already enforces locals=0 and fixes `break`.
Tested on v5.10 on rx3600 machine (ia64 9040 CPU).
CC: Oleg Nesterov <oleg@redhat.com>
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Dmitry V. Levin <ldv@altlinux.org>
Bug: https://bugs.gentoo.org/769614
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
arch/ia64/kernel/ptrace.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
index c3490ee2daa5..e14f5653393a 100644
--- a/arch/ia64/kernel/ptrace.c
+++ b/arch/ia64/kernel/ptrace.c
@@ -2013,27 +2013,39 @@ static void syscall_get_set_args_cb(struct unw_frame_info *info, void *data)
{
struct syscall_get_set_args *args = data;
struct pt_regs *pt = args->regs;
- unsigned long *krbs, cfm, ndirty;
+ unsigned long *krbs, cfm, ndirty, nlocals, nouts;
int i, count;
if (unw_unwind_to_user(info) < 0)
return;
+ /*
+ * We get here via a few paths:
+ * - break instruction: cfm is shared with caller.
+ * syscall args are in out= regs, locals are non-empty.
+ * - epsinstruction: cfm is set by br.call
+ * locals don't exist.
+ *
+ * For both cases argguments are reachable in cfm.sof - cfm.sol.
+ * CFM: [ ... | sor: 17..14 | sol : 13..7 | sof : 6..0 ]
+ */
cfm = pt->cr_ifs;
+ nlocals = (cfm >> 7) & 0x7f; /* aka sol */
+ nouts = (cfm & 0x7f) - nlocals; /* aka sof - sol */
krbs = (unsigned long *)info->task + IA64_RBS_OFFSET/8;
ndirty = ia64_rse_num_regs(krbs, krbs + (pt->loadrs >> 19));
count = 0;
if (in_syscall(pt))
- count = min_t(int, args->n, cfm & 0x7f);
+ count = min_t(int, args->n, nouts);
+ /* Iterate over outs. */
for (i = 0; i < count; i++) {
+ int j = ndirty + nlocals + i + args->i;
if (args->rw)
- *ia64_rse_skip_regs(krbs, ndirty + i + args->i) =
- args->args[i];
+ *ia64_rse_skip_regs(krbs, j) = args->args[i];
else
- args->args[i] = *ia64_rse_skip_regs(krbs,
- ndirty + i + args->i);
+ args->args[i] = *ia64_rse_skip_regs(krbs, j);
}
if (!args->rw) {
--
2.30.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-02-21 0:25 [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
@ 2021-02-21 0:25 ` Sergei Trofimovich
2021-02-21 9:21 ` John Paul Adrian Glaubitz
` (2 more replies)
2021-03-02 23:40 ` [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
2021-03-03 21:51 ` Dmitry V. Levin
2 siblings, 3 replies; 10+ messages in thread
From: Sergei Trofimovich @ 2021-02-21 0:25 UTC (permalink / raw)
To: Andrew Morton, linux-kernel
Cc: Sergei Trofimovich, linux-ia64, Dmitry V . Levin
In https://bugs.gentoo.org/769614 Dmitry noticed that
`ptrace(PTRACE_GET_SYSCALL_INFO)` does not return error sign properly.
The bug is in mismatch between get/set errors:
static inline long syscall_get_error(struct task_struct *task,
struct pt_regs *regs)
{
return regs->r10 == -1 ? regs->r8:0;
}
static inline long syscall_get_return_value(struct task_struct *task,
struct pt_regs *regs)
{
return regs->r8;
}
static inline void syscall_set_return_value(struct task_struct *task,
struct pt_regs *regs,
int error, long val)
{
if (error) {
/* error < 0, but ia64 uses > 0 return value */
regs->r8 = -error;
regs->r10 = -1;
} else {
regs->r8 = val;
regs->r10 = 0;
}
}
Tested on v5.10 on rx3600 machine (ia64 9040 CPU).
CC: linux-ia64@vger.kernel.org
CC: linux-kernel@vger.kernel.org
CC: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Dmitry V. Levin <ldv@altlinux.org>
Bug: https://bugs.gentoo.org/769614
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
arch/ia64/include/asm/syscall.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/ia64/include/asm/syscall.h b/arch/ia64/include/asm/syscall.h
index 6c6f16e409a8..0d23c0049301 100644
--- a/arch/ia64/include/asm/syscall.h
+++ b/arch/ia64/include/asm/syscall.h
@@ -32,7 +32,7 @@ static inline void syscall_rollback(struct task_struct *task,
static inline long syscall_get_error(struct task_struct *task,
struct pt_regs *regs)
{
- return regs->r10 == -1 ? regs->r8:0;
+ return regs->r10 == -1 ? -regs->r8:0;
}
static inline long syscall_get_return_value(struct task_struct *task,
--
2.30.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
@ 2021-02-21 9:21 ` John Paul Adrian Glaubitz
2021-02-21 13:08 ` Sergei Trofimovich
2021-03-02 23:39 ` Sergei Trofimovich
2021-03-03 21:44 ` Dmitry V. Levin
2 siblings, 1 reply; 10+ messages in thread
From: John Paul Adrian Glaubitz @ 2021-02-21 9:21 UTC (permalink / raw)
To: Sergei Trofimovich, Andrew Morton, linux-kernel
Cc: linux-ia64, Dmitry V . Levin
Hi Sergei!
On 2/21/21 1:25 AM, Sergei Trofimovich wrote:
> In https://bugs.gentoo.org/769614 Dmitry noticed that
> `ptrace(PTRACE_GET_SYSCALL_INFO)` does not return error sign properly.
> (...)
Do these two patches unbreak gdb on ia64?
And have you, by any chance, managed to get the hpsa driver working again?
Adrian
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer - glaubitz@debian.org
`. `' Freie Universitaet Berlin - glaubitz@physik.fu-berlin.de
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-02-21 9:21 ` John Paul Adrian Glaubitz
@ 2021-02-21 13:08 ` Sergei Trofimovich
0 siblings, 0 replies; 10+ messages in thread
From: Sergei Trofimovich @ 2021-02-21 13:08 UTC (permalink / raw)
To: John Paul Adrian Glaubitz
Cc: Andrew Morton, linux-kernel, linux-ia64, Dmitry V . Levin
On Sun, 21 Feb 2021 10:21:56 +0100
John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote:
> Hi Sergei!
>
> On 2/21/21 1:25 AM, Sergei Trofimovich wrote:
> > In https://bugs.gentoo.org/769614 Dmitry noticed that
> > `ptrace(PTRACE_GET_SYSCALL_INFO)` does not return error sign properly.
> > (...)
>
> Do these two patches unbreak gdb on ia64?
gdb was somewhat working on ia64 for Gentoo. strace was the main
impacted here.
But I did not try anything complicated recently. Anything specific that
breaks for you?
$ uname -r
5.10.0
(even without the patches above)
$ cat c.c
int main(){}
$ gcc c.c -o a -ggdb3
$ gdb --quiet ./a
Reading symbols from ./a...
(gdb) start
Temporary breakpoint 1 at 0x7f2: file c.c, line 1.
Starting program: /home/slyfox/a
Failed to read a valid object file image from memory.
Temporary breakpoint 1, main () at c.c:1
1 int main(){}
(gdb) disassemble
Dump of assembler code for function main:
0x20000008000007f0 <+0>: [MII] mov r2=r12
0x20000008000007f1 <+1>: mov r14=r0;;
=> 0x20000008000007f2 <+2>: mov r8=r14
0x2000000800000800 <+16>: [MIB] mov r12=r2
0x2000000800000801 <+17>: nop.i 0x0
0x2000000800000802 <+18>: br.ret.sptk.many b0;;
End of assembler dump.
(gdb) break *0x2000000800000800
Breakpoint 2 at 0x2000000800000800: file c.c, line 1.
(gdb) continue
Continuing.
Breakpoint 2, 0x2000000800000800 in main () at c.c:1
1 int main(){}
Looks ok for minor stuff.
> And have you, by any chance, managed to get the hpsa driver working again?
v5.10 seems to boot off hpsa just fine without extra patches:
14:01.0 RAID bus controller: Hewlett-Packard Company Smart Array P600
Subsystem: Hewlett-Packard Company 3 Gb/s SAS RAID
Kernel driver in use: hpsa
v5.11 does not boot yet. Kernel does not see some files while boots after init is
started (but I'm not sure it's a block device problem). Bisecting now why.
--
Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
2021-02-21 9:21 ` John Paul Adrian Glaubitz
@ 2021-03-02 23:39 ` Sergei Trofimovich
2021-03-03 14:30 ` Oleg Nesterov
2021-03-03 21:44 ` Dmitry V. Levin
2 siblings, 1 reply; 10+ messages in thread
From: Sergei Trofimovich @ 2021-03-02 23:39 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, linux-ia64, Dmitry V . Levin, Oleg Nesterov
On Sun, 21 Feb 2021 00:25:54 +0000
Sergei Trofimovich <slyfox@gentoo.org> wrote:
> In https://bugs.gentoo.org/769614 Dmitry noticed that
> `ptrace(PTRACE_GET_SYSCALL_INFO)` does not return error sign properly.
>
> The bug is in mismatch between get/set errors:
>
> static inline long syscall_get_error(struct task_struct *task,
> struct pt_regs *regs)
> {
> return regs->r10 == -1 ? regs->r8:0;
> }
>
> static inline long syscall_get_return_value(struct task_struct *task,
> struct pt_regs *regs)
> {
> return regs->r8;
> }
>
> static inline void syscall_set_return_value(struct task_struct *task,
> struct pt_regs *regs,
> int error, long val)
> {
> if (error) {
> /* error < 0, but ia64 uses > 0 return value */
> regs->r8 = -error;
> regs->r10 = -1;
> } else {
> regs->r8 = val;
> regs->r10 = 0;
> }
> }
>
> Tested on v5.10 on rx3600 machine (ia64 9040 CPU).
>
> CC: linux-ia64@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Dmitry V. Levin <ldv@altlinux.org>
> Bug: https://bugs.gentoo.org/769614
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
> arch/ia64/include/asm/syscall.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/ia64/include/asm/syscall.h b/arch/ia64/include/asm/syscall.h
> index 6c6f16e409a8..0d23c0049301 100644
> --- a/arch/ia64/include/asm/syscall.h
> +++ b/arch/ia64/include/asm/syscall.h
> @@ -32,7 +32,7 @@ static inline void syscall_rollback(struct task_struct *task,
> static inline long syscall_get_error(struct task_struct *task,
> struct pt_regs *regs)
> {
> - return regs->r10 == -1 ? regs->r8:0;
> + return regs->r10 == -1 ? -regs->r8:0;
> }
>
> static inline long syscall_get_return_value(struct task_struct *task,
> --
> 2.30.1
>
Andrew, would it be fine to pass it through misc tree?
Or should it go through Oleg as it's mostly about ptrace?
--
Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls
2021-02-21 0:25 [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
@ 2021-03-02 23:40 ` Sergei Trofimovich
2021-03-03 21:51 ` Dmitry V. Levin
2 siblings, 0 replies; 10+ messages in thread
From: Sergei Trofimovich @ 2021-03-02 23:40 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, Oleg Nesterov, linux-ia64, Dmitry V . Levin
On Sun, 21 Feb 2021 00:25:53 +0000
Sergei Trofimovich <slyfox@gentoo.org> wrote:
> In https://bugs.gentoo.org/769614 Dmitry noticed that
> `ptrace(PTRACE_GET_SYSCALL_INFO)` does not work for syscalls called
> via glibc's syscall() wrapper.
>
> ia64 has two ways to call syscalls from userspace: via `break` and via
> `eps` instructions.
>
> The difference is in stack layout:
>
> 1. `eps` creates simple stack frame: no locals, in{0..7} == out{0..8}
> 2. `break` uses userspace stack frame: may be locals (glibc provides
> one), in{0..7} == out{0..8}.
>
> Both work fine in syscall handling cde itself.
>
> But `ptrace(PTRACE_GET_SYSCALL_INFO)` uses unwind mechanism to
> re-extract syscall arguments but it does not account for locals.
>
> The change always skips locals registers. It should not change `eps`
> path as kernel's handler already enforces locals=0 and fixes `break`.
>
> Tested on v5.10 on rx3600 machine (ia64 9040 CPU).
>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: linux-ia64@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Dmitry V. Levin <ldv@altlinux.org>
> Bug: https://bugs.gentoo.org/769614
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
> arch/ia64/kernel/ptrace.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
> index c3490ee2daa5..e14f5653393a 100644
> --- a/arch/ia64/kernel/ptrace.c
> +++ b/arch/ia64/kernel/ptrace.c
> @@ -2013,27 +2013,39 @@ static void syscall_get_set_args_cb(struct unw_frame_info *info, void *data)
> {
> struct syscall_get_set_args *args = data;
> struct pt_regs *pt = args->regs;
> - unsigned long *krbs, cfm, ndirty;
> + unsigned long *krbs, cfm, ndirty, nlocals, nouts;
> int i, count;
>
> if (unw_unwind_to_user(info) < 0)
> return;
>
> + /*
> + * We get here via a few paths:
> + * - break instruction: cfm is shared with caller.
> + * syscall args are in out= regs, locals are non-empty.
> + * - epsinstruction: cfm is set by br.call
> + * locals don't exist.
> + *
> + * For both cases argguments are reachable in cfm.sof - cfm.sol.
> + * CFM: [ ... | sor: 17..14 | sol : 13..7 | sof : 6..0 ]
> + */
> cfm = pt->cr_ifs;
> + nlocals = (cfm >> 7) & 0x7f; /* aka sol */
> + nouts = (cfm & 0x7f) - nlocals; /* aka sof - sol */
> krbs = (unsigned long *)info->task + IA64_RBS_OFFSET/8;
> ndirty = ia64_rse_num_regs(krbs, krbs + (pt->loadrs >> 19));
>
> count = 0;
> if (in_syscall(pt))
> - count = min_t(int, args->n, cfm & 0x7f);
> + count = min_t(int, args->n, nouts);
>
> + /* Iterate over outs. */
> for (i = 0; i < count; i++) {
> + int j = ndirty + nlocals + i + args->i;
> if (args->rw)
> - *ia64_rse_skip_regs(krbs, ndirty + i + args->i) =
> - args->args[i];
> + *ia64_rse_skip_regs(krbs, j) = args->args[i];
> else
> - args->args[i] = *ia64_rse_skip_regs(krbs,
> - ndirty + i + args->i);
> + args->args[i] = *ia64_rse_skip_regs(krbs, j);
> }
>
> if (!args->rw) {
> --
> 2.30.1
>
Andrew, would it be fine to pass it through misc tree?
Or should it go through Oleg as it's about ptrace?
--
Sergei
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-03-02 23:39 ` Sergei Trofimovich
@ 2021-03-03 14:30 ` Oleg Nesterov
2021-03-03 21:40 ` Dmitry V. Levin
0 siblings, 1 reply; 10+ messages in thread
From: Oleg Nesterov @ 2021-03-03 14:30 UTC (permalink / raw)
To: Sergei Trofimovich, Tony Luck, Fenghua Yu
Cc: Andrew Morton, linux-kernel, linux-ia64, Dmitry V . Levin
On 03/02, Sergei Trofimovich wrote:
>
> > --- a/arch/ia64/include/asm/syscall.h
> > +++ b/arch/ia64/include/asm/syscall.h
> > @@ -32,7 +32,7 @@ static inline void syscall_rollback(struct task_struct *task,
> > static inline long syscall_get_error(struct task_struct *task,
> > struct pt_regs *regs)
> > {
> > - return regs->r10 == -1 ? regs->r8:0;
> > + return regs->r10 == -1 ? -regs->r8:0;
> > }
> >
> > static inline long syscall_get_return_value(struct task_struct *task,
> > --
> > 2.30.1
> >
>
> Andrew, would it be fine to pass it through misc tree?
> Or should it go through Oleg as it's mostly about ptrace?
We usually route ptrace fixes via mm tree.
But this fix and another patch from you "ia64: fix ia64_syscall_get_set_arguments()
for break-based syscalls" look very much ia64 specific. I don't think it's actually
about ptrace, and I didn't even try to review these patches because I do not
understand this low level ia64 code.
Can it be routed via ia64 tree? Add Tony and Fenghua...
Oleg.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-03-03 14:30 ` Oleg Nesterov
@ 2021-03-03 21:40 ` Dmitry V. Levin
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry V. Levin @ 2021-03-03 21:40 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Sergei Trofimovich, Tony Luck, Fenghua Yu, Andrew Morton,
linux-kernel, linux-ia64
On Wed, Mar 03, 2021 at 03:30:19PM +0100, Oleg Nesterov wrote:
> On 03/02, Sergei Trofimovich wrote:
> >
> > > --- a/arch/ia64/include/asm/syscall.h
> > > +++ b/arch/ia64/include/asm/syscall.h
> > > @@ -32,7 +32,7 @@ static inline void syscall_rollback(struct task_struct *task,
> > > static inline long syscall_get_error(struct task_struct *task,
> > > struct pt_regs *regs)
> > > {
> > > - return regs->r10 == -1 ? regs->r8:0;
> > > + return regs->r10 == -1 ? -regs->r8:0;
> > > }
> > >
> > > static inline long syscall_get_return_value(struct task_struct *task,
> > > --
> > > 2.30.1
> > >
> >
> > Andrew, would it be fine to pass it through misc tree?
> > Or should it go through Oleg as it's mostly about ptrace?
>
> We usually route ptrace fixes via mm tree.
>
> But this fix and another patch from you "ia64: fix ia64_syscall_get_set_arguments()
> for break-based syscalls" look very much ia64 specific. I don't think it's actually
> about ptrace, and I didn't even try to review these patches because I do not
> understand this low level ia64 code.
>
> Can it be routed via ia64 tree? Add Tony and Fenghua...
Apparently [1], ia64 architecture is now orphaned, so we don't have this
option anymore.
[1] https://git.kernel.org/torvalds/c/96ec72a3425d1515b69b7f9dc34a4a6ce5862a37
--
ldv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
2021-02-21 9:21 ` John Paul Adrian Glaubitz
2021-03-02 23:39 ` Sergei Trofimovich
@ 2021-03-03 21:44 ` Dmitry V. Levin
2 siblings, 0 replies; 10+ messages in thread
From: Dmitry V. Levin @ 2021-03-03 21:44 UTC (permalink / raw)
To: Andrew Morton; +Cc: Sergei Trofimovich, linux-kernel, linux-ia64
On Sun, Feb 21, 2021 at 12:25:54AM +0000, Sergei Trofimovich wrote:
> In https://bugs.gentoo.org/769614 Dmitry noticed that
> `ptrace(PTRACE_GET_SYSCALL_INFO)` does not return error sign properly.
>
> The bug is in mismatch between get/set errors:
>
> static inline long syscall_get_error(struct task_struct *task,
> struct pt_regs *regs)
> {
> return regs->r10 == -1 ? regs->r8:0;
> }
>
> static inline long syscall_get_return_value(struct task_struct *task,
> struct pt_regs *regs)
> {
> return regs->r8;
> }
>
> static inline void syscall_set_return_value(struct task_struct *task,
> struct pt_regs *regs,
> int error, long val)
> {
> if (error) {
> /* error < 0, but ia64 uses > 0 return value */
> regs->r8 = -error;
> regs->r10 = -1;
> } else {
> regs->r8 = val;
> regs->r10 = 0;
> }
> }
>
> Tested on v5.10 on rx3600 machine (ia64 9040 CPU).
>
> CC: linux-ia64@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Dmitry V. Levin <ldv@altlinux.org>
> Bug: https://bugs.gentoo.org/769614
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
> arch/ia64/include/asm/syscall.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/ia64/include/asm/syscall.h b/arch/ia64/include/asm/syscall.h
> index 6c6f16e409a8..0d23c0049301 100644
> --- a/arch/ia64/include/asm/syscall.h
> +++ b/arch/ia64/include/asm/syscall.h
> @@ -32,7 +32,7 @@ static inline void syscall_rollback(struct task_struct *task,
> static inline long syscall_get_error(struct task_struct *task,
> struct pt_regs *regs)
> {
> - return regs->r10 == -1 ? regs->r8:0;
> + return regs->r10 == -1 ? -regs->r8:0;
> }
>
> static inline long syscall_get_return_value(struct task_struct *task,
Reviewed-by: Dmitry V. Levin <ldv@altlinux.org>
--
ldv
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls
2021-02-21 0:25 [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
2021-03-02 23:40 ` [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
@ 2021-03-03 21:51 ` Dmitry V. Levin
2 siblings, 0 replies; 10+ messages in thread
From: Dmitry V. Levin @ 2021-03-03 21:51 UTC (permalink / raw)
To: Andrew Morton; +Cc: Sergei Trofimovich, linux-kernel, Oleg Nesterov, linux-ia64
On Sun, Feb 21, 2021 at 12:25:53AM +0000, Sergei Trofimovich wrote:
> In https://bugs.gentoo.org/769614 Dmitry noticed that
> `ptrace(PTRACE_GET_SYSCALL_INFO)` does not work for syscalls called
> via glibc's syscall() wrapper.
>
> ia64 has two ways to call syscalls from userspace: via `break` and via
> `eps` instructions.
>
> The difference is in stack layout:
>
> 1. `eps` creates simple stack frame: no locals, in{0..7} == out{0..8}
> 2. `break` uses userspace stack frame: may be locals (glibc provides
> one), in{0..7} == out{0..8}.
>
> Both work fine in syscall handling cde itself.
>
> But `ptrace(PTRACE_GET_SYSCALL_INFO)` uses unwind mechanism to
> re-extract syscall arguments but it does not account for locals.
>
> The change always skips locals registers. It should not change `eps`
> path as kernel's handler already enforces locals=0 and fixes `break`.
>
> Tested on v5.10 on rx3600 machine (ia64 9040 CPU).
>
> CC: Oleg Nesterov <oleg@redhat.com>
> CC: linux-ia64@vger.kernel.org
> CC: linux-kernel@vger.kernel.org
> CC: Andrew Morton <akpm@linux-foundation.org>
> Reported-by: Dmitry V. Levin <ldv@altlinux.org>
> Bug: https://bugs.gentoo.org/769614
> Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
> ---
> arch/ia64/kernel/ptrace.c | 24 ++++++++++++++++++------
> 1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/arch/ia64/kernel/ptrace.c b/arch/ia64/kernel/ptrace.c
> index c3490ee2daa5..e14f5653393a 100644
> --- a/arch/ia64/kernel/ptrace.c
> +++ b/arch/ia64/kernel/ptrace.c
> @@ -2013,27 +2013,39 @@ static void syscall_get_set_args_cb(struct unw_frame_info *info, void *data)
> {
> struct syscall_get_set_args *args = data;
> struct pt_regs *pt = args->regs;
> - unsigned long *krbs, cfm, ndirty;
> + unsigned long *krbs, cfm, ndirty, nlocals, nouts;
> int i, count;
>
> if (unw_unwind_to_user(info) < 0)
> return;
>
> + /*
> + * We get here via a few paths:
> + * - break instruction: cfm is shared with caller.
> + * syscall args are in out= regs, locals are non-empty.
> + * - epsinstruction: cfm is set by br.call
> + * locals don't exist.
typo: epsinstruction
> + *
> + * For both cases argguments are reachable in cfm.sof - cfm.sol.
typo: argguments
> + * CFM: [ ... | sor: 17..14 | sol : 13..7 | sof : 6..0 ]
> + */
> cfm = pt->cr_ifs;
> + nlocals = (cfm >> 7) & 0x7f; /* aka sol */
> + nouts = (cfm & 0x7f) - nlocals; /* aka sof - sol */
> krbs = (unsigned long *)info->task + IA64_RBS_OFFSET/8;
> ndirty = ia64_rse_num_regs(krbs, krbs + (pt->loadrs >> 19));
>
> count = 0;
> if (in_syscall(pt))
> - count = min_t(int, args->n, cfm & 0x7f);
> + count = min_t(int, args->n, nouts);
>
> + /* Iterate over outs. */
> for (i = 0; i < count; i++) {
> + int j = ndirty + nlocals + i + args->i;
> if (args->rw)
> - *ia64_rse_skip_regs(krbs, ndirty + i + args->i) =
> - args->args[i];
> + *ia64_rse_skip_regs(krbs, j) = args->args[i];
> else
> - args->args[i] = *ia64_rse_skip_regs(krbs,
> - ndirty + i + args->i);
> + args->args[i] = *ia64_rse_skip_regs(krbs, j);
> }
>
> if (!args->rw) {
This stuff is too ia64 specific, so I cannot properly review this patch,
but it definitely fixes ia64 PTRACE_GET_SYSCALL_INFO on entering syscall.
--
ldv
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-03-04 0:11 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-21 0:25 [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
2021-02-21 0:25 ` [PATCH] ia64: fix ptrace(PTRACE_SYSCALL_INFO_EXIT) sign Sergei Trofimovich
2021-02-21 9:21 ` John Paul Adrian Glaubitz
2021-02-21 13:08 ` Sergei Trofimovich
2021-03-02 23:39 ` Sergei Trofimovich
2021-03-03 14:30 ` Oleg Nesterov
2021-03-03 21:40 ` Dmitry V. Levin
2021-03-03 21:44 ` Dmitry V. Levin
2021-03-02 23:40 ` [PATCH] ia64: fix ia64_syscall_get_set_arguments() for break-based syscalls Sergei Trofimovich
2021-03-03 21:51 ` Dmitry V. Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).