linux-kernel - Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas within sighandler

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: <CALCETrXPYLqunBNxjS8bpmpg+jG_MXbSyZtUVK3X3m+pGSQ1Og@mail.gmail.com>
Date:	Sun, 31 Jan 2016 12:11:10 -0800
From:	Andy Lutomirski <luto@...capital.net>
To:	Stas Sergeev <stsp@...t.ru>
Cc:	Linux kernel <linux-kernel@...r.kernel.org>,
	Linux API <linux-api@...r.kernel.org>,
	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	"Amanieu d'Antras" <amanieu@...il.com>,
	Richard Weinberger <richard@....at>, Tejun Heo <tj@...nel.org>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Jason Low <jason.low2@...com>,
	Heinrich Schuchardt <xypron.glpk@....de>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Konstantin Khlebnikov <khlebnikov@...dex-team.ru>,
	Josh Triplett <josh@...htriplett.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Aleksa Sarai <cyphar@...har.com>,
	Paul Moore <pmoore@...hat.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Vladimir Davydov <vdavydov@...allels.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 4/4] sigaltstack: allow disabling and re-enabling sas
 within sighandler

On Sun, Jan 31, 2016 at 12:08 PM, Stas Sergeev <stsp@...t.ru> wrote:
> 31.01.2016 22:03, Andy Lutomirski пишет:
>> Also, consider a use case like yours but with *two* contexts that use
>> their own altstack.  If you go to context A, enable sigaltstack, get a
>> signal, temporarily disable, then swapcontext to B, which tries to
>> re-enable its own sigaltstack, then everything gets confusing with
>> your patch, because, with your patch, the kernel is only tracking one
>> temporarily disabled sigaltstack.
>
> Of course the good practice is to set the sigaltstack
> before creating the contexts. Then the above scenario
> should involve switching between 2 signal handlers to get
> into troubles. I think the scenario with switching between
> 2 signal handlers is very-very unrealistic.

Why is it so unrealistic?  You're already using swapcontext, which
means you're doing something like userspace threads (although I
imagine that one of your thread-like things is DOS, but still), and,
to me, that suggests that the kernel interface should be agnostic as
to how many thread-like thinks are alive.

With your patch, where the kernel remembers that you have a
temporarily disabled altstack, you can't swap out your context on one
kernel thread and swap it in on another, and you can't have two
different contexts that get used on the same thread.

ISTM it would be simpler if you did:

sigaltstack(disable, force)
swapcontext() to context using sigaltstack
sigaltstack(set new altstack)

and then later

sigaltstack(disable, force)  /* just in case.  save old state, too. */
swapcontext() to context not using sigaltstack
sigaltstack(set new altstack)

If it would be POSIX compliant to allow SS_DISABLE to work even if on
the altstack even without a new flag (which is what you're
suggesting), then getting rid of the temporary in-kernel state would
considerably simplify this patch series.  Just skip the -EPERM check
in the disable path.

--Andy