![]() |
|
Message-ID: <20171111001627.GS1627@brightrain.aerifal.cx> Date: Fri, 10 Nov 2017 19:16:27 -0500 From: Rich Felker <dalias@...c.org> To: musl@...ts.openwall.com Subject: Re: [PATCH] save/restore errno around pthread_atfork handlers On Fri, Nov 10, 2017 at 06:03:40PM -0600, Bobby Bingham wrote: > On Fri, Nov 10, 2017 at 06:31:34PM -0500, Rich Felker wrote: > > 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. > > Good point. It does make me wonder though: when libc invokes a callback > and that callback sets errno to zero, is that a violation of the > prohibition on library functions setting errno to zero? No, that's application code setting it to 0. The case I'm talking about is when errno is 0 before fork is called, 0 gets stored in olderr, the atfork handler sets errno to some nonzero value, and then the implementation wrongly sets it back to 0. That's observable by the application. > > 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? > > Yes, and it's also probably simpler. I'll send a new patch. OK. Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.