From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/12078 Path: news.gmane.org!.POSTED!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [PATCH] save/restore errno around pthread_atfork handlers Date: Fri, 10 Nov 2017 18:31:34 -0500 Message-ID: <20171110233134.GR1627@brightrain.aerifal.cx> References: <20171110205829.18319-1-koorogi@koorogi.info> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: blaine.gmane.org 1510356706 27432 195.159.176.226 (10 Nov 2017 23:31:46 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Fri, 10 Nov 2017 23:31:46 +0000 (UTC) User-Agent: Mutt/1.5.21 (2010-09-15) To: musl@lists.openwall.com Original-X-From: musl-return-12094-gllmg-musl=m.gmane.org@lists.openwall.com Sat Nov 11 00:31:43 2017 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.84_2) (envelope-from ) id 1eDIly-0006yf-Qh for gllmg-musl@m.gmane.org; Sat, 11 Nov 2017 00:31:42 +0100 Original-Received: (qmail 10055 invoked by uid 550); 10 Nov 2017 23:31:47 -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 10034 invoked from network); 10 Nov 2017 23:31:46 -0000 Content-Disposition: inline In-Reply-To: <20171110205829.18319-1-koorogi@koorogi.info> Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:12078 Archived-At: On Fri, Nov 10, 2017 at 02:58:29PM -0600, Bobby Bingham wrote: > If the syscall fails, errno must be preserved for the caller. There's no > guarantee that the handlers registered with pthread_atfork won't clobber > errno. > --- > src/process/fork.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/src/process/fork.c b/src/process/fork.c > index b96f0024..6602eafc 100644 > --- a/src/process/fork.c > +++ b/src/process/fork.c > @@ -15,6 +15,7 @@ pid_t fork(void) > { > pid_t ret; > sigset_t set; > + int olderr; > __fork_handler(-1); > __block_all_sigs(&set); > #ifdef SYS_fork > @@ -30,6 +31,10 @@ pid_t fork(void) > libc.threads_minus_1 = 0; > } > __restore_sigs(&set); > + > + olderr = errno; > __fork_handler(!ret); > + errno = olderr; > + > return ret; > } > -- > 2.15.0 I think the patch as written is incorrect, because it can set errno to 0 after application code in the atfork handler set it to something nonzero; doing so is non-conforming. It would be possible to special-case to avoid this, but it probably makes more sense to just call SYS_fork/SYS_clone with __syscall rather than syscall, then return __syscall_ret(ret) instead of return ret. Does that sound correct? Rich