2009/10/21 Eliot Miranda eliot.miranda@gmail.com:
Hi All, I'm not happy with this fix and I'm not happy with the lack of process behind it. First there has been insufficient discussion of what the right behaviour is. Second, the fix David has written does lots of computation (shifts) to check a valid size request that could be pushed earlier at initialization time, which would allow e.g. a vmParameterAt:put: to modify the max allocation request size. Third, there is no review of fixes; we just put them out there. I'm concerned about performance, code quality and a lack of process for agreeing fixes. But at the same time I don't want to institute a bureaucracy or slow down the pace of development. Do others share my concerns? What suggestions have you?
One problem here is that Cog will introduce a huge raft of changes to the VM and to Slang, and so possibly the whole issue is moot. We'll face the issues as we try and integrate my Cog VM into the squeakvm trunk. But it might be worth thinking a little about the issues up front. David, I know you're technical lead, and I'm not trying to depose or undermine you. But I do think we can benefit from discussion and review of major changes. Alas, my Cog work not being generaly available yet is going to cause problems down the line. I need to at least hurry up and get the StackInterpreter released.
Hi Eliot & Dave.
I think that Dave's fix is a quick way to close the security hole. I mean, it is good to have some critical issues closed quickly and deliver the 'hot' fix than having no fix at all. And surely, it should stay open for further discussion how to make it better/cleaner/faster/safer etc..
I hope nothing in this fix is unrevertable, which can't be changed in future versions.
On Tue, Oct 20, 2009 at 7:39 PM, David T. Lewis lewis@mail.msen.com wrote:
On Tue, Oct 06, 2009 at 06:58:30PM -0400, David T. Lewis wrote:
On Tue, Oct 06, 2009 at 10:05:02PM +0200, Nicolas Cellier wrote:
Is this known?
Thanks, it's known now :)
Also added to Mantis at http://bugs.squeak.org/view.php?id=7405.
Petr: Yes this is definitely a cool bug.
A fix for this is on Mantis 7405 (http://bugs.squeak.org/view.php?id=7405).
The VMMaker updates are in SqueakSource in VMMaker-dtl.142.
A separate patch is needed for platforms/unix/vm/sqUnixMemory.c (see the Mantis report, patch also sent to Ian).
I have not tested Windows or Mac OS, but the basic scenario now is that the VM will limit the size of memory increase requests to 31 bit integer so support code can check for overflows. The requests from the VM are guaranteed (I hope) be valid 31 bit integers.
Allocation requests of > 950 MB seem to be possible, although I do not have enough memory on my PC to verify that it actually works.
Limiting allocation requests to more reasonable limits would be the responsibility of the image.
Dave