[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: mlock/guard page/xen-tools & lenny



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 :-(.
> 
> 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 <torvalds@linux-foundation.org>
        Date:   Fri Aug 20 16:49:40 2010 -0700
        
            mm: make stack guard page logic use vm_prev pointer
            
            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 <ijc@hellion.org.uk>
            Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
        
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;
 }

-- 
Ian Campbell
Current Noise: Bryan Ferry & Roxy Music - Do The Strand

The farther you go, the less you know.
		-- Lao Tsu, "Tao Te Ching"


Reply to: