https://bugzilla.wikimedia.org/show_bug.cgi?id=68196
--- Comment #18 from Brad Jorsch <[email protected]> --- So if I understand the part Ori was asking me about today, we're still wanting to look at the memory limit handling as Tim talked about in comment 12? If the below sounds fine to you, Tim, I can have code in Gerrit tomorrow. (In reply to Tim Starling from comment #12) > Also, because Lua is in the stack, the in_lua flag is set, so "slop" is > zero. That is to say, the hack intended to fix bug 59130 is disabled. The > Lua userspace takes the usage to within 36 bytes of the limit, and > lua_pushcclosure() tries to allocate 40, so after the error is handled, it's > assured that re-entering the same state will cause an unprotected OOM. > > in_lua and in_php are both flags indicating the contents of the stack, not > the immediate caller. Maybe we need a third flag which indicates that the > immediate caller is unprotected. The fix for bug 59130 was just checking for whether the call was protected at some level, which in_lua indicates. But it sounds like the problem here is that we can't have Lua error handling jumping back to an outer level of recursion, so we need to consider protectedness at each level instead of just globally. I don't think we need a third flag: we start out unprotected (with in_lua=0 and in_php=0). When we enter protected mode, that's luasandbox_call_helper incrementing in_lua and doing a lua_pcall. A recursive call from Lua back to PHP is going to go through luasandbox_call_php, which increments in_php, and then calling back into Lua will increment in_lua again. So it should be the case that in_lua == in_php+1 whenever we're protected and in_lua == in_php whenever we're not. If we were to add a "protected" flag, we'd basically replicate that logic: clear at start, save-and-set it before the pcall then restore it after in luasandbox-call_helper, and save-and-clear it before calling into PHP and then restore it after in luasandbox_call_php. I put in asserts testing the relationship between in_lua and in_php, and both the luasandbox tests and the Scribunto tests all passed locally (on Zend, but that shouldn't make a difference) without tripping them. > Also, I think the 1 MB slop might be a bit too conservative. Since the > result of an OOM is a crash, we should probably just disable the memory > limit entirely for unprotected calls. I chose 1MB because it seemed like it would be reasonable for what the unprotected calls were doing, while still having some limitation in case someone figured out some way to make an unprotected call use large amounts of memory. But if that assumption is wrong, it's easy enough to disable the limit entirely. Would we still keep the prevention of a single allocation greater than the memory limit, or disable that one too? -- You are receiving this mail because: You are the assignee for the bug. You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
