linux-kernel - Re: [PATCH] signal: always clear sa

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]

Date:	Mon, 11 Mar 2013 12:34:39 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Al Viro <viro@...iv.linux.org.uk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Serge Hallyn <serge.hallyn@...onical.com>,
	Emese Revfy <re.emese@...il.com>,
	PaX Team <pageexec@...email.hu>
Subject: Re: [PATCH] signal: always clear sa_restorer on execve

On Mon, Mar 11, 2013 at 12:28 PM, Oleg Nesterov <oleg@...hat.com> wrote:
> On 03/11, Kees Cook wrote:
>>
>> When the new signal handlers are set up for a fork, the location of
>> sa_restorer is not cleared, leaking a parent process's address space
>> location to children. This allows for a potential bypass of the parent's
>> ASLR by examining the sa_restorer value returned when calling sigaction().
>
> I don't understand.
>
> fork() should not change restorer/etc, and the child has the same address
> space anyway. There is no any leak and the patch can't make any difference
> in this case because flush_signal_handlers() is not called by fork().

I probably failed to explain this correctly. From the perspective of
what should be considered "secret", it only matters across the exec,
not the fork (since the VMAs haven't changed until the exec). But the
info leak is easy to see, and this patch fixes it. As you say, since
other things were reset, so should sa_restorer.

>
>> @@ -485,6 +485,9 @@ flush_signal_handlers(struct task_struct *t, int force_default)
>>               if (force_default || ka->sa.sa_handler != SIG_IGN)
>>                       ka->sa.sa_handler = SIG_DFL;
>>               ka->sa.sa_flags = 0;
>> +#ifdef __ARCH_HAS_SA_RESTORER
>> +             ka->sa.sa_restorer = NULL;
>> +#endif
>
> However, exec sets SIG_DFL but keeps ->sa_restorer, so probably this
> patch makes sense anyway.
>
> Oleg.
>

-Kees


-- 
Kees Cook
Chrome OS Security
--
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/