Re: Is the registry needed?

[William Ahern <william@...>]

On Thu, Oct 01, 2015 at 11:55:18AM -0400, Sean Conner wrote:
> It was thus said that the Great Patrick Donnelly once stated:
> > On Wed, Sep 30, 2015 at 4:54 PM, Sean Conner <sean@conman.org> wrote:
> > > It was thus said that the Great Soni L. once stated:
> > >> This is VERY unsafe! You're potentially breaking a sandbox by doing a
> > >> sethook. And you don't reset it, either.
> > >
> > >   If you mean "I don't call lua_sethook(L,NULL,0,0)" then you have misread
> > > the code---I do call that.  But if you mean "I don't restore the previous
> > > hooks" then you're right, I don't.  When I first started this implementation
> > > of the signal module, I was concerned about that.  And I tried.  And it was
> > > very fragile.  I then decided to check how other signal modules handled this
> > > and guess what?  NONE OF THEM RESTORE THE HOOKS!  Not even the standalone
> > > Lua interpeter bothers with restoring any hooks.
> > 
> > https://github.com/batrick/lua-signal/blob/49b2fccf546ca0096af6aca26052eb0981e28f59/lsignal.c#L167-L211
> 
>   Oh!  Well ... um ... okay.  
> 

That code seems broken to me.

1) hook processes outstanding signals
2) hook loads NULL from old_hook.hook at line 196
3) signal handler is invoked
4) handle loads NULL from old_hook.hook at line 203
5) handle calls lua_sethook at line 208
6) handle updates signal_stack at line 210
7) signal handler exits
8) hook invokes lua_sethook with NULL hook argument and exits
9) hook is never run again

The obvious and simplest fix, I think, is to block all signals in the hook.

At a minimum I would call sigfillset on the sigaction mask at line 292.
Analysis is needlessly complicated by multiple signals.

The increments aren't atomic, even on x86. It's probably benign, but also
unnecessary.

FWIW, l_pause is totally broken, but it looks like an afterthought, anyhow.
If a signal arrives after entering l_pause but before calling sigprocmask,
the process could hang. It should run any outstanding Lua hooks after
calling sigprocmask. Also, it never restores the old signal mask.