On Tue 04-07-17 17:51:40, Willy Tarreau wrote: > On Tue, Jul 04, 2017 at 12:36:11PM +0100, Ben Hutchings wrote: > > > If anywhing this would require to have a loop over all PROT_NONE > > > mappings to not hit into other weird usecases. > > > > That's what I was thinking of. Tried the following patch: > (...) > > - next = vma->vm_next; > > + /* > > + * Allow VM_NONE mappings in the gap as some applications try > > + * to make their own stack guards > > + */ > > + for (next = vma->vm_next; > > + next && !(next->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)); > > + next = next->vm_next) > > + ; > > That's what I wanted to propose but I feared someone would scream at me > for this loop :-)
Well, I've been thinking about this some more and the more I think about it the less I am convinced we should try to be clever here. Why? Because as soon as somebody tries to manage stacks explicitly you cannot simply assume anything about the previous mapping. Say some interpret uses [ mngmnt data][red zone] <--[- MAP_GROWSDOWN ] Now if we consider the red zone's (PROT_NONE) prev mapping we would fail the expansion even though we haven't hit the red zone and that is essentially what the Java and rust bugs are about. So we just risk yet another regression. Now let's say another example <--[- MAP_GROWSDOWN][red zone] <--[- MAP_GROWSDOWN] thread 1 thread 2 Does the more clever code prevent from smashing over unrelated stack? No because of our VM_GROWS{DOWN,UP} checks which are needed for other cases. Well we could special case those as well but... That being said, I am not really convinced that mixing 2 different gap implemetantions is sane. I guess it should be reasonable to assume that a PROT_NONE mapping close to the stack is meant to be a red zone and at this moment we should rather back off and rely on the userspace rather than risk more weird cornercases and regressions. -- Michal Hocko SUSE Labs