From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/14368 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] Fix the use of sigaltstack to return to the saved main stack. Date: Tue, 9 Jul 2019 15:30:04 -0400 Message-ID: <20190709193004.GQ1506@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="247049"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-14384-gllmg-musl=m.gmane.org@lists.openwall.com Tue Jul 09 21:30:21 2019 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by blaine.gmane.org with smtp (Exim 4.89) (envelope-from ) id 1hkvoh-0012A1-JZ for gllmg-musl@m.gmane.org; Tue, 09 Jul 2019 21:30:19 +0200 Original-Received: (qmail 22227 invoked by uid 550); 9 Jul 2019 19:30:17 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 22180 invoked from network); 9 Jul 2019 19:30:16 -0000 Content-Disposition: inline In-Reply-To: Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:14368 Archived-At: On Tue, Jul 09, 2019 at 03:01:50PM -0400, James Y Knight wrote: > Previously, musl would reject the call, because the main stack has > ss_size == 0 and ss_flags == SS_DISABLE. > > We could condition on ss_flags not containing SS_DISABLE, but instead, > simply remove the ss_size check, as the kernel performs the same check, > anyhow. Are you sure the kernel does? I'm pretty sure the reason the code is here is that the kernel either does not check it, or does not perform the check correctly in some special case. Sadly the commit messages in musl were not as good back at the time when the code was written. Unless we have good evidence the test isn't needed, I think the right check is just making the error conditional on !(ss_flags & SS_DISABLE). POSIX specifies: "If it is set to SS_DISABLE, the stack is disabled and ss_sp and ss_size are ignored." Here, "set to" is probably something the resolution of Austin Group issue 1187 failed to fix; it should probably be "includes" rather than "is set to". But I'm not sure it makes sense to have any flags set alongside SS_DISABLE anyway. > [[Note: I've also included the corresponding patch to libc-tests. I'm not > sure if this is OK, or if I should send it separately?]] I think that's fine. Rich > From fe70a508fe945cb1a44f8a6bbd87ee295637447b Mon Sep 17 00:00:00 2001 > From: James Y Knight > Date: Tue, 9 Jul 2019 14:59:01 -0400 > Subject: [PATCH] Fix the use of sigaltstack to return to the saved main stack. > > Previously, musl would reject the call, because the main stack has > ss_size == 0 and ss_flags == SS_DISABLE. > > We could condition on ss_flags not containing SS_DISABLE, but instead, > simply remove the ss_size check, as the kernel performs the same check, > anyhow. > --- > src/signal/sigaltstack.c | 12 +++--------- > 1 file changed, 3 insertions(+), 9 deletions(-) > > diff --git a/src/signal/sigaltstack.c b/src/signal/sigaltstack.c > index cfa3f5c1..d8e8eb0b 100644 > --- a/src/signal/sigaltstack.c > +++ b/src/signal/sigaltstack.c > @@ -4,15 +4,9 @@ > > int sigaltstack(const stack_t *restrict ss, stack_t *restrict old) > { > - if (ss) { > - if (ss->ss_size < MINSIGSTKSZ) { > - errno = ENOMEM; > - return -1; > - } > - if (ss->ss_flags & SS_ONSTACK) { > - errno = EINVAL; > - return -1; > - } > + if (ss && (ss->ss_flags & SS_ONSTACK)) { > + errno = EINVAL; > + return -1; > } > return syscall(SYS_sigaltstack, ss, old); > } > -- > 2.22.0.410.gd8fdbe21b5-goog > > From 9272ed8666f930dc2f24d05b1fd248584bbf495f Mon Sep 17 00:00:00 2001 > From: James Y Knight > Date: Tue, 9 Jul 2019 14:31:24 -0400 > Subject: [PATCH] Verify that returning to the original stack doesn't return an > error (e.g. ENOMEM). > > --- > src/regression/sigaltstack.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/regression/sigaltstack.c b/src/regression/sigaltstack.c > index bfdc44a..6847454 100644 > --- a/src/regression/sigaltstack.c > +++ b/src/regression/sigaltstack.c > @@ -30,7 +30,7 @@ static void handler(int sig) > > int main(void) > { > - stack_t ss; > + stack_t ss, oldss; > struct sigaction sa; > > ss.ss_sp = stack; > @@ -39,7 +39,7 @@ int main(void) > sa.sa_handler = handler; > sa.sa_flags = SA_ONSTACK; > > - T(sigaltstack(&ss, 0)); > + T(sigaltstack(&ss, &oldss)); > T(sigfillset(&sa.sa_mask)); > T(sigaction(SIGUSR1, &sa, 0)); > T(raise(SIGUSR1)); > @@ -56,7 +56,7 @@ int main(void) > t_error("sigaltstack with bad ss_flags should have failed with EINVAL, " > "got %s\n", strerror(errno)); > errno = 0; > - T(sigaltstack(0, 0)); > + T(sigaltstack(oldss, 0)); > > return t_status; > } > -- > 2.22.0.410.gd8fdbe21b5-goog >