Re: [PATCH 1/4] mlockall(MCL_FUTURE) unlocks currently locked mappings

[Chris Wright <chrisw@osdl.org>]

* Chris Wright (chrisw@osdl.org) wrote:
> * Andrew Morton (akpm@osdl.org) wrote:
> > I've always assumed that mlockall(MCL_FUTURE) pins all your current pages
> > as well as future ones.  But no, that's what MCL_CURRENT|MCL_FUTURE does.
> > 
> > So when we fix this bug, we'll break my buggy test apps.
> > 
> > I wonder what other apps we'll break?
> 
> I don't think it will break apps.  The only difference is that it won't
> unlock already locked mappings.

For example, I have a test that locks some pages, then calls
mlockall(MCL_FUTURE).  Resutls in unlocking all locked pages.

# pages locked, before MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:      1244 kB
# after MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:         0 kB

And, with just a simple call to MCL_FUTURE:
# before MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:         0 kB
# after MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:         0 kB

Now with the patch:

# pages locked, before MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:      1244 kB
# after MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:      1244 kB
	    ^^^^^^^
That's the only differenece, and I believe fixes a real bug whose
existance is more likely to surprise apps than it's squashing ;-)

And, again (unchanged) with just a simple call to MCL_FUTURE:
# before MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:         0 kB
# after MCL_FUTURE
$ cat /proc/`pidof mlockall`/status | grep VmLck
VmLck:         0 kB

The crux of the problem is in do_mlockall() in the for loop:

                newflags = vma->vm_flags | VM_LOCKED;
                if (!(flags & MCL_CURRENT))
                        newflags &= ~VM_LOCKED;
                mlock_fixup(vma, vma->vm_start, vma->vm_end, newflags);

So, for flags == MCL_FUTURE only, might as well bail out before the loop,
since MCL_FUTURE does nothing w.r.t. current mappings.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net