linux-kernel - Re: [PATCH] x86: make PTRACE_GETREGSET return 32-bit regs if 64-bit process entered kernel with int 80

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130214150046.GA30543@redhat.com>
Date:	Thu, 14 Feb 2013 16:00:46 +0100
From:	Oleg Nesterov <oleg@...hat.com>
To:	Denys Vlasenko <dvlasenk@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Andi Kleen <andi@...stfloor.org>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86: make PTRACE_GETREGSET return 32-bit regs if
	64-bit process entered kernel with int 80

On 02/14, Denys Vlasenko wrote:
>
> Determining personality of a ptraced process is a murky area.
> On x86, for years strace was looking at segment selectors,
> which is conceptually wrong: see, for example,
> https://lkml.org/lkml/2012/1/18/320
>
> strace recently changed detection method and current git code
> (not released yet) does the following: it reads registers
> with PTRACE_GETREGSET, and looks at returned regset size.
> It is different for 64-bit and 32-bit processes,
> and appears to be a reliable way to determine personality:
> No need to check segment selectors for magic values.
>
> This works for well-behaving processes.
>
> But the hole described in the aforementioned lkml thread
> still remains: 64-bit processes can perform 32-bit syscalls
> using "int 80" entry method, and in this case, kernel returns
> 64-bit regset. For example, this:
>
> asm("int $0x80": :"a" (29)); /* 32-bit sys_pause */
>
> will be decoded by strace as a (64-bit) shmget syscall.
>
> This patch makes it so that in syscall-entry-stop caused by
> "int 80" instruction, PTRACE_GETREGSET returns 32-bit regset.

Not sure...

First of all, this is incompatible change. And to me, it doesn't look
correct anyway. Say, why the debugger can't modify r15 if a 64bit tracee
does int80 ? Or think about PTRACE_EVENT_FORK which can be reported with
TS_COMPAT set.

Probably is_ia32_task() should be reported "explicitely" as we already
discussed, and afaik you have other ideas.

Oleg.

> Signed-off-by: Denys Vlasenko <dvlasenk@...hat.com>
>
> --- linux-3.7.7/arch/x86/include/asm/uaccess.h
> +++ linux-3.7.7_regset/arch/x86/include/asm/uaccess.h
> @@ -594,5 +594,7 @@
>  # include <asm/uaccess_64.h>
>  #endif
>
> +#define ARCH_HAS_SYSCALL_USER_REGSET_VIEW 1
> +
>  #endif /* _ASM_X86_UACCESS_H */
>
> --- linux-3.7.7/arch/x86/kernel/ptrace.c
> +++ linux-3.7.7_regset/arch/x86/kernel/ptrace.c
> @@ -1443,6 +1443,22 @@
>  #endif
>  }
>
> +const struct user_regset_view *syscall_user_regset_view(struct task_struct *task)
> +{
> +#ifdef CONFIG_IA32_EMULATION
> +	/* Did task make 32-bit syscall just now?
> +	 * Task can still be 64-bit: think "int 0x80 on x86_64".
> +	 */
> +	if (task_thread_info(task)->status & TS_COMPAT)
> +#endif
> +#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
> +		return &user_x86_32_view;
> +#endif
> +#ifdef CONFIG_X86_64
> +	return &user_x86_64_view;
> +#endif
> +}
> +
>  static void fill_sigtrap_info(struct task_struct *tsk,
>  				struct pt_regs *regs,
>  				int error_code, int si_code,
> --- linux-3.7.7/include/linux/regset.h
> +++ linux-3.7.7_regset/include/linux/regset.h
> @@ -204,6 +204,12 @@
>   */
>  const struct user_regset_view *task_user_regset_view(struct task_struct *tsk);
>
> +#ifdef ARCH_HAS_SYSCALL_USER_REGSET_VIEW
> +const struct user_regset_view *syscall_user_regset_view(struct task_struct *tsk);
> +#else
> +# define syscall_user_regset_view task_user_regset_view
> +#endif
> +
>
>  /*
>   * These are helpers for writing regset get/set functions in arch code.
> --- linux-3.7.7/kernel/ptrace.c
> +++ linux-3.7.7_regset/kernel/ptrace.c
> @@ -684,7 +684,7 @@
>  static int ptrace_regset(struct task_struct *task, int req, unsigned int type,
>  			 struct iovec *kiov)
>  {
> -	const struct user_regset_view *view = task_user_regset_view(task);
> +	const struct user_regset_view *view = syscall_user_regset_view(task);
>  	const struct user_regset *regset = find_regset(view, type);
>  	int regset_no;
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/