lua-users home
lua-l archive

[Date Prev][Date Next][Thread Prev][Thread Next] [Date Index] [Thread Index]

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 <> 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.
> > 
> >
>   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

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.