[BUG][PATCH] 2.4.* mlockall(MCL_FUTURE) is broken -- child inherits VM_LOCKED

[BUG][PATCH] 2.4.* mlockall(MCL_FUTURE) is broken -- child inherits VM_LOCKED

[Dave Anderson]

In 2.4.*, mlockall(MCL_FUTURE) is erroneously inherited by child processes
across fork() and exec():

 1. across a fork(), the inherited memory is not locked, but any new memory
    allocations by the child will be VM_LOCKED.
 2. across a subsequent exec(), *all* of the exec'd program's memory except
    for its stack pages will be VM_LOCKED.

The problem is:

 1.  if MCL_FUTURE, mm->def_flags gets set to VM_LOCKED in do_mlockall().
 2.  mm->def_flags is not cleared during subsequent forks and execs.
 3   mm->def_flags, with the leftover VM_LOCKED flag set, is subsequently
     utilized in calc_vm_flags() when called by do_brk() to extend the
     address space of a forked process, and by do_mmap_pgoff() when
     building the non-stack address space of an exec'd process.

The proposed patch puts the fix in mm_init(), which seems to be the most
appropriate place since it's called by copy_mm(), and by mm_alloc() on behalf
of exec_mmap():


# diff -u linux/kernel/fork.c linux-2.4.17/kernel/fork.c
--- linux/kernel/fork.c Tue Jan  8 15:11:13 2002
+++ linux-2.4.17/kernel/fork.c  Tue Jan  8 15:12:26 2002
@@ -219,6 +219,7 @@
        init_rwsem(&mm->mmap_sem);
        mm->page_table_lock = SPIN_LOCK_UNLOCKED;
        mm->pgd = pgd_alloc(mm);
+       mm->def_flags = 0;
        if (mm->pgd)
                return mm;
        free_mm(mm);


Note that it worked OK in 2.2 because mm->def_flags was explicitly cleared in
mm_alloc(), which was called by both copy_mm() and exec_mmap().  But things
were shuffled around a bit in 2.4, and it must have gotten lost in the
translation...

Dave Anderson

==============================================================================
  David Anderson                             anderson@mclinux.com
  Mission Critical Linux, Inc.               http://www.mclinux.com
  100 Foot of John St.                       Work: 978-606-0225
  Lowell, MA 01852                           Fax: 978-446-9470
==============================================================================

Re: [BUG][PATCH] 2.4.* mlockall(MCL_FUTURE) is broken -- child inherits VM_LOCKED

[Andrew Morton]

Dave Anderson wrote:
> 
> In 2.4.*, mlockall(MCL_FUTURE) is erroneously inherited by child processes
> across fork() and exec():

The Linux manpage says that it is not inherited across either.

However SUS says that it is not inherited across exec, and
doesn't mention fork() at all.
http://www.opengroup.org/onlinepubs/007908799/xsh/mlockall.html

So...  Shouldn't we be clearing it in the exec() path?

> ...
> # diff -u linux/kernel/fork.c linux-2.4.17/kernel/fork.c
> --- linux/kernel/fork.c Tue Jan  8 15:11:13 2002
> +++ linux-2.4.17/kernel/fork.c  Tue Jan  8 15:12:26 2002
> @@ -219,6 +219,7 @@
>         init_rwsem(&mm->mmap_sem);
>         mm->page_table_lock = SPIN_LOCK_UNLOCKED;
>         mm->pgd = pgd_alloc(mm);
> +       mm->def_flags = 0;
>         if (mm->pgd)
>                 return mm;
>         free_mm(mm);
> 
> Note that it worked OK in 2.2 because mm->def_flags was explicitly cleared in
> mm_alloc(), which was called by both copy_mm() and exec_mmap().  But things
> were shuffled around a bit in 2.4, and it must have gotten lost in the
> translation...

um.  Is this correct?  It seems that we'll be clearing things
like VM_IO on device mappings across fork.  Bad.  Would an explicit
clear of VM_LOCKED be better here?  (Assuming we want to ignore SUS).

-

Re: [BUG][PATCH] 2.4.* mlockall(MCL_FUTURE) is broken -- child inherits VM_LOCKED

[Bruce Blinn]

Andrew Morton wrote:
> 
> Dave Anderson wrote:
> >
> > In 2.4.*, mlockall(MCL_FUTURE) is erroneously inherited by child processes
> > across fork() and exec():
> 
> The Linux manpage says that it is not inherited across either.
> 
> However SUS says that it is not inherited across exec, and
> doesn't mention fork() at all.
> http://www.opengroup.org/onlinepubs/007908799/xsh/mlockall.html
> 
> So...  Shouldn't we be clearing it in the exec() path?
> 

But, the SUS documentation for fork() says that it does not inherit the
memory locks of the parent.  It explicitly mentions mlockall().

	http://www.opengroup.org/onlinepubs/007908799/xsh/fork.html

Re: [BUG][PATCH] 2.4.* mlockall(MCL_FUTURE) is broken -- child inherits VM_LOCKED

[Andrew Morton]

Bruce Blinn wrote:
> 
> Andrew Morton wrote:
> >
> > Dave Anderson wrote:
> > >
> > > In 2.4.*, mlockall(MCL_FUTURE) is erroneously inherited by child processes
> > > across fork() and exec():
> >
> > The Linux manpage says that it is not inherited across either.
> >
> > However SUS says that it is not inherited across exec, and
> > doesn't mention fork() at all.
> > http://www.opengroup.org/onlinepubs/007908799/xsh/mlockall.html
> >
> > So...  Shouldn't we be clearing it in the exec() path?
> >
> 
> But, the SUS documentation for fork() says that it does not inherit the
> memory locks of the parent.  It explicitly mentions mlockall().
> 
>         http://www.opengroup.org/onlinepubs/007908799/xsh/fork.html

So it does.  So clearing it on fork is correct.  And my comment
regarding def_flags was nonsense.  Probably it's best to explicitly
clear VM_LOCKED, just in case something else gets added to def_flags
in the future.

Apart from that - ship it :)

-