Skip to content

Commit

Permalink
um: Fix ptrace GETREGS/SETREGS bugs
Browse files Browse the repository at this point in the history
This fix two related bugs:
* PTRACE_GETREGS doesn't get the right orig_ax (syscall) value
* PTRACE_SETREGS can't set the orig_ax value (erased by initial value)

Get rid of the now useless and error-prone get_syscall().

Fix inconsistent behavior in the ptrace implementation for i386 when
updating orig_eax automatically update the syscall number as well. This
is now updated in handle_syscall().

Signed-off-by: Mickaël Salaün <[email protected]>
Cc: Jeff Dike <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Will Drewry <[email protected]>
Cc: Thomas Meyer <[email protected]>
Cc: Nicolas Iooss <[email protected]>
Cc: Anton Ivanov <[email protected]>
Cc: Meredydd Luff <[email protected]>
Cc: David Drysdale <[email protected]>
Signed-off-by: Richard Weinberger <[email protected]>
Acked-by: Kees Cook <[email protected]>
  • Loading branch information
l0kod authored and richardweinberger committed Jan 10, 2016
1 parent a7df471 commit e04c989
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 25 deletions.
1 change: 0 additions & 1 deletion arch/um/include/shared/os.h
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,6 @@ extern void initial_thread_cb_skas(void (*proc)(void *),
void *arg);
extern void halt_skas(void);
extern void reboot_skas(void);
extern int get_syscall(struct uml_pt_regs *regs);

/* irq.c */
extern int os_waiting_for_events(struct irq_fd *active_fds);
Expand Down
26 changes: 14 additions & 12 deletions arch/um/kernel/skas/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,31 @@
#include <linux/ptrace.h>
#include <kern_util.h>
#include <sysdep/ptrace.h>
#include <sysdep/ptrace_user.h>
#include <sysdep/syscalls.h>
#include <os.h>

void handle_syscall(struct uml_pt_regs *r)
{
struct pt_regs *regs = container_of(r, struct pt_regs, regs);
long result;
int syscall;

if (syscall_trace_enter(regs)) {
result = -ENOSYS;
/* Initialize the syscall number and default return value. */
UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
PT_REGS_SET_SYSCALL_RETURN(regs, -ENOSYS);

if (syscall_trace_enter(regs))
goto out;
}

syscall = get_syscall(r);
/* Update the syscall number after orig_ax has potentially been updated
* with ptrace.
*/
UPT_SYSCALL_NR(r) = PT_SYSCALL_NR(r->gp);
syscall = UPT_SYSCALL_NR(r);

if ((syscall > __NR_syscall_max) || syscall < 0)
result = -ENOSYS;
else
result = EXECUTE_SYSCALL(syscall, regs);
if (syscall >= 0 && syscall <= __NR_syscall_max)
PT_REGS_SET_SYSCALL_RETURN(regs,
EXECUTE_SYSCALL(syscall, regs));

out:
PT_REGS_SET_SYSCALL_RETURN(regs, result);

syscall_trace_leave(regs);
}
7 changes: 0 additions & 7 deletions arch/um/os-Linux/skas/process.c
Original file line number Diff line number Diff line change
Expand Up @@ -172,13 +172,6 @@ static void handle_trap(int pid, struct uml_pt_regs *regs,
handle_syscall(regs);
}

int get_syscall(struct uml_pt_regs *regs)
{
UPT_SYSCALL_NR(regs) = PT_SYSCALL_NR(regs->gp);

return UPT_SYSCALL_NR(regs);
}

extern char __syscall_stub_start[];

static int userspace_tramp(void *stack)
Expand Down
8 changes: 3 additions & 5 deletions arch/x86/um/ptrace_32.c
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ static const int reg_offsets[] = {
[EFL] = HOST_EFLAGS,
[UESP] = HOST_SP,
[SS] = HOST_SS,
[ORIG_EAX] = HOST_ORIG_AX,
};

int putreg(struct task_struct *child, int regno, unsigned long value)
Expand All @@ -83,6 +84,7 @@ int putreg(struct task_struct *child, int regno, unsigned long value)
case EAX:
case EIP:
case UESP:
case ORIG_EAX:
break;
case FS:
if (value && (value & 3) != 3)
Expand All @@ -108,9 +110,6 @@ int putreg(struct task_struct *child, int regno, unsigned long value)
value &= FLAG_MASK;
child->thread.regs.regs.gp[HOST_EFLAGS] |= value;
return 0;
case ORIG_EAX:
child->thread.regs.regs.syscall = value;
return 0;
default :
panic("Bad register in putreg() : %d\n", regno);
}
Expand Down Expand Up @@ -143,8 +142,6 @@ unsigned long getreg(struct task_struct *child, int regno)

regno >>= 2;
switch (regno) {
case ORIG_EAX:
return child->thread.regs.regs.syscall;
case FS:
case GS:
case DS:
Expand All @@ -163,6 +160,7 @@ unsigned long getreg(struct task_struct *child, int regno)
case EDI:
case EBP:
case EFL:
case ORIG_EAX:
break;
default:
panic("Bad register in getreg() : %d\n", regno);
Expand Down

0 comments on commit e04c989

Please sign in to comment.