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: [email protected]
CC: [email protected]
CC: Andrew Morton <[email protected]>
Reported-by: Dmitry V. Levin <[email protected]>
Bug: https://bugs.gentoo.org/769614
Signed-off-by: Sergei Trofimovich <[email protected]>
---
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
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 - [email protected]
`. `' Freie Universitaet Berlin - [email protected]
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913
On Sun, 21 Feb 2021 10:21:56 +0100
John Paul Adrian Glaubitz <[email protected]> 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
On Sun, 21 Feb 2021 00:25:54 +0000
Sergei Trofimovich <[email protected]> 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: [email protected]
> CC: [email protected]
> CC: Andrew Morton <[email protected]>
> Reported-by: Dmitry V. Levin <[email protected]>
> Bug: https://bugs.gentoo.org/769614
> Signed-off-by: Sergei Trofimovich <[email protected]>
> ---
> 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
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.
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: [email protected]
> CC: [email protected]
> CC: Andrew Morton <[email protected]>
> Reported-by: Dmitry V. Levin <[email protected]>
> Bug: https://bugs.gentoo.org/769614
> Signed-off-by: Sergei Trofimovich <[email protected]>
> ---
> 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 <[email protected]>
--
ldv
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