Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:29588
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 63.205.162.114 as permitted sender)
Message-ID: <4651351D.8010306@zend.com>
Date: Sun, 20 May 2007 22:58:53 -0700
Organization: Zend Technologies
User-Agent: Thunderbird 2.0.0.0 (Windows/20070326)
MIME-Version: 1.0
To: Stefan Esser <sesser@hardened-php.net>, 
 'PHP Internals' <internals@lists.php.net>
References: <465022BE.1020905@hardened-php.net>
In-Reply-To: <465022BE.1020905@hardened-php.net>
Content-Type: text/plain; charset=ISO-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Dismantling the lies...
From: stas@zend.com (Stanislav Malyshev)

> In the other thread, where Stanislav spreads the usual lie/propaganda
> that there is no help comming from me to the PHP developers, he also
> claims

For the record - I never said that. I said that in that thread, as
in some of its predecessors, when it came from general discussion to the
substance Stefan somehow lost interest to the substance part. Of course,
he's not in any obligation to contribute to PHP and his substantial help
- when it indeed happens - is always welcome. I just noted in that
particular case it didn't happen, regretfully. Now it happens, which I 
can only be glad about.

> that the MOPB issues #1 and #2 cannot be fixed right now. Xdebug,
> Suhosin, Hardening Patch have already demonstrated for years that it
> is not true that #2 [2] cannot be fixed without breaking binary 
> compatibility.

Actually #1 can't be, as I see it. #2 can, but that requires arbitrarily
limiting stack depth as I far as I can see.

> There have also been patches on this mailinglist that calculated the
> maximum depth automatically, therefore there is no need to dismantle
> this lie.

I don't imagine how one really could calculate maximum depth without
solving the halting problem, so I must be missing something. I ask
somebody who knows what these patches are to send me a link - if there 
were patches that do that automatically for any code I would very much 
like to see them.
Adding the arbitrary - even configurable - limit doesn't seem to me the
correct solution, since it has high potential to break application code
- with the same effect as the actual bug would. In the context of the
debugger it might be OK since the developer would get immediate feedback
and thus could know what and why crashed, in the context of runtime
application that could be as bad as original bug. Also, since stack size 
differs for different OSes, it would be rather hard to find good default 
for it. BTW, as far as I can understand, Hardened PHP patch has this 
protection disabled by default.

> First of all everyone into PHP development knows that the obvious
> "fix" for this issue would be to just break binary compatibility and
> use a 32 bit reference counter. It does not fix the actual problem
> but it is enough so that it cannot be triggered anymore.

Yes, this is correct. However, that would require releasing
binary-incompatible PHP 4 version, which is not supported by the group
as I understand.

> However there exists another fix to the problem that deals with the 
> actual problem of an overflowing reference counter. Therefore every
> refcount increase in the Zend Engine Source has to be protected.
> While this sounds much of work it actually takes less than half an
> hour to do it.

That would also incur considerable performance penalty. As I see in your 
patch, you convert a number of increments (which would normally generate 
single increment instruction) into function calls. Also, it doesn't 
cover extensions accessing refcounts directly (and they do that) - which 
is less of a problem, those could be converted too. But performance loss 
makes me worry. Did you test the performance difference for the patch?
Even if function calls are converted to a macro, it still might have 
some performance lost since refcount change is a very frequent operation 
in the engine and inserting a branch for it could hurt performance. 
However, if I see somebody showing the patch with near-zero effect it 
can be a good case for including such patch.

-- 
Stanislav Malyshev, Zend Products Engineer
stas@zend.com  http://www.zend.com/