Re: Last patch

Re: Last patch

[Sven Wischnowsky]

I wrote:

> I'll have a look at the weekend. Let's see if we can do it with
> signal-queueing.

I had.  It's a nightmare.  I haven't yet checked everything, but the
patch already has ~3600 lines.  The main problem seems to be that we
have many places where we get, for example, a pointer to a hash node,
keep it in a local variable and later use it.  If a trap would be
handle between getting the pointer and using it, the handler could do
something that invalidates the hash table entry (and hence the
pointer).

In many places this can quite easily be wrapped inside a queue_signals()/
unqueue_signals() pair but there are places where it is extremly
difficult (execcmd(), for example, or the function-autoloading code).

Another problem is the module code.  Most of this could probably be
solved by queue/unqueue pairs (I've done/tried that) and by
disallowing to unload modules from a handler.

Some (if not most) of the things I haven't tried to make secure yet
include the history code and the job- and signal-tables. But even
without them, the many queue/unqueue pairs don't look like the right
solution.  Maybe we should try to identify a hopefully small set of
places where we put them, each making a possibly large part of the
shell secure and later try to move the pairs further and further down
the call chains.


Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: Last patch

[Bart Schaefer]

On Dec 4, 10:06am, Sven Wischnowsky wrote:
} 
} I wrote:
} 
} > I'll have a look at the weekend. Let's see if we can do it with
} > signal-queueing.
} 
} I had.  It's a nightmare.  I haven't yet checked everything, but the
} patch already has ~3600 lines.
[...]
} Some (if not most) of the things I haven't tried to make secure yet
} include the history code and the job- and signal-tables. But even
} without them, the many queue/unqueue pairs don't look like the right
} solution.  Maybe we should try to identify a hopefully small set of
} places where we put them, each making a possibly large part of the
} shell secure and later try to move the pairs further and further down
} the call chains.

Yes, that would be my suggestion, with some careful choosing of the large
parts.  (I'm sorry that I don't have time right now to do much more than
offer advice about this -- I'm the busiest with work right now that I've
been for some months.)

Another thing is that we should recognize that zsh has been working
reasonably well without this for a very long time.  If we can pick off
the places where the shell spends most of its CPU cycles, we can make
what problems there are, far less likely.  Then worry about covering
the rest of the cases as we have opportunity.

} The main problem seems to be that we
} have many places where we get, for example, a pointer to a hash node,
} keep it in a local variable and later use it.
[...]
} In many places this can quite easily be wrapped inside a queue_signals()/
} unqueue_signals() pair but there are places where it is extremly
} difficult (execcmd(), for example, or the function-autoloading code).

At least the two examples you gave are cases where I think it would be
OK to queue signals over a wider span of code.  Some signals are blocked
for a good portion of execcmd() already, aren't they?  (To avoid SIGCHLD
races.)

} Another problem is the module code.  Most of this could probably be
} solved by queue/unqueue pairs (I've done/tried that) and by
} disallowing to unload modules from a handler.

Couldn't attempting unload from a handler be treated the same as from
a wrapper, i.e., just set the MOD_UNLOAD flag on the module struct?

-- 
Bart Schaefer                                 Brass Lantern Enterprises
http://www.well.com/user/barts              http://www.brasslantern.com

Zsh: http://www.zsh.org | PHPerl Project: http://phperl.sourceforge.net   

Re: Last patch

[Sven Wischnowsky]

Peter Stephenson wrote:

> Just realised after I sent it that the diff for installing functions
> contained Bart's tweak for forbidding traps during traps, which hasn't been
> committed.  The traps are the last piece of major unfinished business I
> know about.  (I am not volunteering to anything about it.)

I'll have a look at the weekend. Let's see if we can do it with
signal-queueing.

As I already said, the globals in the completion system shouldn't be a 
problem if we just prevent it from being called while it is still
active (I haven't looked yet, maybe we already do that -- I have the
impression that I shoudl now this, hm). The globals in zle: for now we 
only need to protect those that can be touched by using the `zle'
builtin. The stuff used for the lexer/parser/execution code shouldn't
be a problem either because they are already saved and restored. And
for the rest we could probably start by making the protected section
somewhat larger than necessary (the functions used in the hashtable
structs might be good candidates, for example).


And then there is also the issue of moving the shell code into the
final directory structure.


Is our goal to bring out 4.0 before christmas? (`zsh christmas edition'?)
Sounds like a good goal and considering that we got only few bug
reports lately and quite a few people already seem to use 3.1.x...

Bye
 Sven


--
Sven Wischnowsky                         wischnow@informatik.hu-berlin.de

Re: Last patch

[Peter Stephenson]

Sven wrote:
> Is our goal to bring out 4.0 before christmas? (`zsh christmas edition'?)
> Sounds like a good goal and considering that we got only few bug
> reports lately and quite a few people already seem to use 3.1.x...

It would be a good goal to aim for, and if this trap business can be
resolved probably possible.  There's nothing else I can think of that might
need the relatively long settling period we had with 3.1.7..9.

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
Cambridge Silicon Radio, Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070

Last patch

[Peter Stephenson]

Just realised after I sent it that the diff for installing functions
contained Bart's tweak for forbidding traps during traps, which hasn't been
committed.  The traps are the last piece of major unfinished business I
know about.  (I am not volunteering to anything about it.)

-- 
Peter Stephenson <pws@csr.com>                  Software Engineer
Cambridge Silicon Radio, Unit 300, Science Park, Milton Road,
Cambridge, CB4 0XL, UK                          Tel: +44 (0)1223 392070