linux-kernel - Re: [PATCH v3 3/3] x86/ptrace, x86/signal: Remove TS_I386_REGS

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: <20160623212606.GB14180@redhat.com>
Date:	Thu, 23 Jun 2016 23:26:06 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Andy Lutomirski <luto@...nel.org>
Cc:	x86@...nel.org, linux-kernel@...r.kernel.org,
	Borislav Petkov <bp@...en8.de>,
	Pedro Alves <palves@...hat.com>,
	Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v3 3/3] x86/ptrace, x86/signal: Remove
	TS_I386_REGS_POKED

Again, I think the patch is fine, just a question

On 06/20, Andy Lutomirski wrote:
>
> System call restart has some oddities wrt ptrace:
>
> 1. For whatever reason, the kernel delivers signals and triggers
>    ptrace before handling syscall restart.  This means that
>    -ERESTART_RESTARTBLOCK, etc is visible to userspace.  We could
>    plausibly get away with changing that, but it seems quite risky.

How we can change this?

The kernel simply can't know how it should react to (say) -ERESTARTSYS
until debugger acks/nacks/changes the signal reported by tracee.

> +	/*
> +	 * A 32-bit ptracer has the following expectations:
> +	 *
> +	 * - Storing -1 (i.e. 0xffffffff) to orig_eax will prevent
> +	 *   syscall restart handling.
> +	 *
> +	 * - Restoring regs saved on exit from an interrupted
> +	 *   restartable syscall will trigger syscall restart.  Such
> +	 *   regs will have non-negative orig_eax and negative eax.
> +	 *
> +	 * The kernel's syscall restart code treats regs->orig_ax and
> +	 * regs->ax as 64-bit signed quantities.  32-bit user code
> +	 * doesn't care about the high bits.  Keep it simple and just
> +	 * sign-extend both values.
> +	 */
> +	R32_SIGNED(orig_eax, orig_ax);
> +	R32_SIGNED(eax, ax);

OK. but do we really need R32_SIGNED(orig_eax) ? syscall_get_nr()
returns "int", not "long".

Oleg.