On Tue, Aug 31, 2010 at 11:58:32AM +0100, Ian Campbell wrote: > On Tue, 2010-08-31 at 11:28 +0100, Ian Campbell wrote: > > On Tue, 2010-08-31 at 11:00 +0100, Ian Campbell wrote: > > > On Mon, 2010-08-30 at 22:57 -0600, dann frazier wrote: > > > > On Mon, Aug 30, 2010 at 08:22:23AM -0600, dann frazier wrote: > > > > > hey Ian, > > > > > I haven't seen any reports of xen problems w/ the latest lenny DSA > > > > > that added the guard page code. I looked at backporting the 3 patches > > > > > from Linus - but the mlock patch touches code that didn't exist in > > > > > .26, so I'm wondering if that patch is just not needed. > > > > > > > > fyi, I setup a Xen/lenny system and didn't have any problems w/ > > > > save/restore, so we're good afaict :) > > > > > > Thanks, I can't see code in the Lenny kernel equivalent to that which > > > was broken either. > > > > > > I wrote a pretty skanky test program to test for the issue (mlock.c, > > > attached, run with "./mlock lock" to trigger the issue). I don't have a > > > Lenny system to hand right not where I can run it but if you still have > > > a Lenny system setup then it might be interesting to try. (the test > > > program doesn't actually require Xen to demonstrate the issue so any > > > Lenny system should do). > > > > I installed up a Lenny VM and ran the test case there and it did > > fail :-(.
Thanks for the test case, that's very helpful. I can reproduce. 2.6.26-24 does not fail, 2.6.26-24lenny1 does. > > However, unless we come across a report of an actual failure with the > > real toolstack I don't think it is worth worrying about. > > FWIW I think the below is the moral equivalent of: > commit 0e8e50e20c837eeec8323bba7dcd25fe5479194c > Author: Linus Torvalds <[email protected]> > Date: Fri Aug 20 16:49:40 2010 -0700 > > mm: make stack guard page logic use vm_prev pointer I've actually already included a backport for this in 2.6.26-25 (in p-u), and I've verified that your test case does not fail: da...@dl380g5:~$ ./mlock lock pad1 at 0xffc15380-0xffc1737f LCK at 0xffc17380-0xffc17384 pad2 at 0xffc17384-0xffc19383 esp: 0xffc15360 locking 0xffc17380-0xffc17384 -> 0xffc17000-0xffc18000 -> 0 minor faults: 157 -> 157 major faults: 0 -> 0 So looks like we're ok? -dann > Like the mlock() change previously, this makes the stack guard > check > code use vma->vm_prev to see what the mapping below the current > stack > is, rather than have to look it up with find_vma(). > > Also, accept an abutting stack segment, since that happens > naturally if > you split the stack with mlock or mprotect. > > Tested-by: Ian Campbell <[email protected]> > Signed-off-by: Linus Torvalds <[email protected]> > > without the reliance on vm_prev (IOW it implements only the "Also, ... > bit". I'm just building a test kernel to check it now. > > $ cat > debian/patches/bugfix/all/mm-stack-guard-accept-abutting-stack-segment.patch > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -2287,11 +2287,18 @@ > { > address &= PAGE_MASK; > if ((vma->vm_flags & VM_GROWSDOWN) && address == vma->vm_start) { > - address -= PAGE_SIZE; > - if (find_vma(vma->vm_mm, address) != vma) > - return -ENOMEM; > + struct vm_area_struct *prev = find_vma(vma->vm_mm, address - > PAGE_SIZE); > > - expand_stack(vma, address); > + /* > + * Is there a mapping abutting this one below? > + * > + * That's only ok if it's the same stack mapping > + * that has gotten split.. > + */ > + if (prev && prev->vm_end == address) > + return prev->vm_flags & VM_GROWSDOWN ? 0 : -ENOMEM; > + > + expand_stack(vma, address - PAGE_SIZE); > } > return 0; > } > -- dann frazier -- To UNSUBSCRIBE, email to [email protected] with a subject of "unsubscribe". Trouble? Contact [email protected] Archive: http://lists.debian.org/[email protected]
