diff mbox series

[3/3] mm: check for VMA being detached before destroying it

Message ID 20230620235726.3873043-3-surenb@google.com
State New
Headers
Series [1/3] mm: change vma_start_read to fail if VMA got detached from under it |

Commit Message

Suren Baghdasaryan June 20, 2023, 11:57 p.m. UTC 
  By the time VMA is freed it has to be detached with the exception of
exit_mmap which is destroying the whole VMA tree. Enforce this
requirement before freeing the VMA. exit_mmap in the only user calling
__vm_area_free directly, therefore it won't trigger the new check.
Change VMA initialization to mark new VMAs as detached and change that
flag once the VMA is added into a tree.

Suggested-by: Linus Torvalds <torvalds@linuxfoundation.org>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
 include/linux/mm.h | 4 ++--
 kernel/fork.c      | 2 ++
 mm/internal.h      | 1 +
 3 files changed, 5 insertions(+), 2 deletions(-)

Comments

kernel test robot June 21, 2023, 2:15 a.m. UTC | #1 
Hi Suren,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Suren-Baghdasaryan/mm-change-vma_start_read-to-fail-to-lock-a-detached-VMA/20230621-075833
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230620235726.3873043-3-surenb%40google.com
patch subject: [PATCH 3/3] mm: check for VMA being detached before destroying it
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230621/202306211007.hQoEsMrP-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230621/202306211007.hQoEsMrP-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306211007.hQoEsMrP-lkp@intel.com/

All errors (new ones prefixed by >>):

   scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
   scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
   scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
   In file included from include/linux/pid_namespace.h:7,
                    from include/linux/ptrace.h:10,
                    from arch/alpha/kernel/asm-offsets.c:11:
   include/linux/mm.h: In function 'vma_init':
>> include/linux/mm.h:753:12: error: 'struct vm_area_struct' has no member named 'detached'
     753 |         vma->detached = true;
         |            ^~
   arch/alpha/kernel/asm-offsets.c: At top level:
   arch/alpha/kernel/asm-offsets.c:15:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
      15 | void foo(void)
         |      ^~~
   make[2]: *** [scripts/Makefile.build:114: arch/alpha/kernel/asm-offsets.s] Error 1
   make[2]: Target 'prepare' not remade because of errors.
   make[1]: *** [Makefile:1287: prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:226: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.


vim +753 include/linux/mm.h

   740	
   741	/*
   742	 * WARNING: vma_init does not initialize vma->vm_lock.
   743	 * Use vm_area_alloc()/vm_area_free() if vma needs locking.
   744	 */
   745	static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
   746	{
   747		static const struct vm_operations_struct dummy_vm_ops = {};
   748	
   749		memset(vma, 0, sizeof(*vma));
   750		vma->vm_mm = mm;
   751		vma->vm_ops = &dummy_vm_ops;
   752		INIT_LIST_HEAD(&vma->anon_vma_chain);
 > 753		vma->detached = true;
   754		vma_numab_state_init(vma);
   755	}
   756
kernel test robot June 21, 2023, 5:53 a.m. UTC | #2 
Hi Suren,

kernel test robot noticed the following build errors:

[auto build test ERROR on akpm-mm/mm-everything]

url:    https://github.com/intel-lab-lkp/linux/commits/Suren-Baghdasaryan/mm-change-vma_start_read-to-fail-to-lock-a-detached-VMA/20230621-075833
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20230620235726.3873043-3-surenb%40google.com
patch subject: [PATCH 3/3] mm: check for VMA being detached before destroying it
config: sparc-randconfig-r022-20230620 (https://download.01.org/0day-ci/archive/20230621/202306211345.Xt94CVTt-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230621/202306211345.Xt94CVTt-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306211345.Xt94CVTt-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/sound/pcm.h:15,
                    from sound/drivers/aloop.c:27:
   include/linux/mm.h: In function 'vma_init':
   include/linux/mm.h:753:12: error: 'struct vm_area_struct' has no member named 'detached'
     753 |         vma->detached = true;
         |            ^~
   In file included from include/linux/init.h:5,
                    from sound/drivers/aloop.c:18:
   sound/drivers/aloop.c: At top level:
>> include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
      16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
         |                                                   ^
   include/linux/compiler.h:231:33: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
     231 | #define __must_be_array(a)      BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
         |                                 ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:56:59: note: in expansion of macro '__must_be_array'
      56 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                                           ^~~~~~~~~~~~~~~
   include/linux/moduleparam.h:517:20: note: in expansion of macro 'ARRAY_SIZE'
     517 |         = { .max = ARRAY_SIZE(array), .num = nump,                      \
         |                    ^~~~~~~~~~
   include/linux/moduleparam.h:501:9: note: in expansion of macro 'module_param_array_named'
     501 |         module_param_array_named(name, name, type, nump, perm)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   sound/drivers/aloop.c:46:1: note: in expansion of macro 'module_param_array'
      46 | module_param_array(index, int, NULL, 0444);
         | ^~~~~~~~~~~~~~~~~~
   sound/drivers/aloop.c:39:12: warning: 'index' defined but not used [-Wunused-variable]
      39 | static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;      /* Index 0-MAX */
         |            ^~~~~
--
   In file included from include/sound/pcm.h:15,
                    from sound/drivers/dummy.c:20:
   include/linux/mm.h: In function 'vma_init':
   include/linux/mm.h:753:12: error: 'struct vm_area_struct' has no member named 'detached'
     753 |         vma->detached = true;
         |            ^~
   In file included from include/linux/init.h:5,
                    from sound/drivers/dummy.c:7:
   sound/drivers/dummy.c: At top level:
>> include/linux/build_bug.h:16:51: error: bit-field '<anonymous>' width not an integer constant
      16 | #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
         |                                                   ^
   include/linux/compiler.h:231:33: note: in expansion of macro 'BUILD_BUG_ON_ZERO'
     231 | #define __must_be_array(a)      BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
         |                                 ^~~~~~~~~~~~~~~~~
   include/linux/kernel.h:56:59: note: in expansion of macro '__must_be_array'
      56 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                                           ^~~~~~~~~~~~~~~
   include/linux/moduleparam.h:517:20: note: in expansion of macro 'ARRAY_SIZE'
     517 |         = { .max = ARRAY_SIZE(array), .num = nump,                      \
         |                    ^~~~~~~~~~
   include/linux/moduleparam.h:501:9: note: in expansion of macro 'module_param_array_named'
     501 |         module_param_array_named(name, name, type, nump, perm)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~
   sound/drivers/dummy.c:62:1: note: in expansion of macro 'module_param_array'
      62 | module_param_array(index, int, NULL, 0444);
         | ^~~~~~~~~~~~~~~~~~
   sound/drivers/dummy.c:48:12: warning: 'index' defined but not used [-Wunused-variable]
      48 | static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX;      /* Index 0-MAX */
         |            ^~~~~


vim +16 include/linux/build_bug.h

bc6245e5efd70c Ian Abbott       2017-07-10   6  
bc6245e5efd70c Ian Abbott       2017-07-10   7  #ifdef __CHECKER__
bc6245e5efd70c Ian Abbott       2017-07-10   8  #define BUILD_BUG_ON_ZERO(e) (0)
bc6245e5efd70c Ian Abbott       2017-07-10   9  #else /* __CHECKER__ */
bc6245e5efd70c Ian Abbott       2017-07-10  10  /*
bc6245e5efd70c Ian Abbott       2017-07-10  11   * Force a compilation error if condition is true, but also produce a
8788994376d84d Rikard Falkeborn 2019-12-04  12   * result (of value 0 and type int), so the expression can be used
bc6245e5efd70c Ian Abbott       2017-07-10  13   * e.g. in a structure initializer (or where-ever else comma expressions
bc6245e5efd70c Ian Abbott       2017-07-10  14   * aren't permitted).
bc6245e5efd70c Ian Abbott       2017-07-10  15   */
8788994376d84d Rikard Falkeborn 2019-12-04 @16  #define BUILD_BUG_ON_ZERO(e) ((int)(sizeof(struct { int:(-!!(e)); })))
527edbc18a70e7 Masahiro Yamada  2019-01-03  17  #endif /* __CHECKER__ */
527edbc18a70e7 Masahiro Yamada  2019-01-03  18
Suren Baghdasaryan June 21, 2023, 7:01 a.m. UTC | #3 
On Tue, Jun 20, 2023 at 7:15 PM kernel test robot <lkp@intel.com> wrote:
>
> Hi Suren,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on akpm-mm/mm-everything]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/Suren-Baghdasaryan/mm-change-vma_start_read-to-fail-to-lock-a-detached-VMA/20230621-075833
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
> patch link:    https://lore.kernel.org/r/20230620235726.3873043-3-surenb%40google.com
> patch subject: [PATCH 3/3] mm: check for VMA being detached before destroying it
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20230621/202306211007.hQoEsMrP-lkp@intel.com/config)
> compiler: alpha-linux-gcc (GCC) 12.3.0
> reproduce: (https://download.01.org/0day-ci/archive/20230621/202306211007.hQoEsMrP-lkp@intel.com/reproduce)
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202306211007.hQoEsMrP-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
>    scripts/genksyms/parse.y: warning: 9 shift/reduce conflicts [-Wconflicts-sr]
>    scripts/genksyms/parse.y: warning: 5 reduce/reduce conflicts [-Wconflicts-rr]
>    scripts/genksyms/parse.y: note: rerun with option '-Wcounterexamples' to generate conflict counterexamples
>    In file included from include/linux/pid_namespace.h:7,
>                     from include/linux/ptrace.h:10,
>                     from arch/alpha/kernel/asm-offsets.c:11:
>    include/linux/mm.h: In function 'vma_init':
> >> include/linux/mm.h:753:12: error: 'struct vm_area_struct' has no member named 'detached'
>      753 |         vma->detached = true;
>          |            ^~

Yep, missed #ifdef CONFIG_PER_VMA_LOCK here. Will fix it in the next
version but will wait a bit for possible feedback.

>    arch/alpha/kernel/asm-offsets.c: At top level:
>    arch/alpha/kernel/asm-offsets.c:15:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
>       15 | void foo(void)
>          |      ^~~
>    make[2]: *** [scripts/Makefile.build:114: arch/alpha/kernel/asm-offsets.s] Error 1
>    make[2]: Target 'prepare' not remade because of errors.
>    make[1]: *** [Makefile:1287: prepare0] Error 2
>    make[1]: Target 'prepare' not remade because of errors.
>    make: *** [Makefile:226: __sub-make] Error 2
>    make: Target 'prepare' not remade because of errors.
>
>
> vim +753 include/linux/mm.h
>
>    740
>    741  /*
>    742   * WARNING: vma_init does not initialize vma->vm_lock.
>    743   * Use vm_area_alloc()/vm_area_free() if vma needs locking.
>    744   */
>    745  static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
>    746  {
>    747          static const struct vm_operations_struct dummy_vm_ops = {};
>    748
>    749          memset(vma, 0, sizeof(*vma));
>    750          vma->vm_mm = mm;
>    751          vma->vm_ops = &dummy_vm_ops;
>    752          INIT_LIST_HEAD(&vma->anon_vma_chain);
>  > 753          vma->detached = true;
>    754          vma_numab_state_init(vma);
>    755  }
>    756
>
> --
> 0-DAY CI Kernel Test Service
> https://github.com/intel/lkp-tests/wiki
diff mbox series

Patch

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 74e3033c9fc2..9a10fcdb134e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -247,7 +247,7 @@  void setup_initial_init_mm(void *start_code, void *end_code,
 struct vm_area_struct *vm_area_alloc(struct mm_struct *);
 struct vm_area_struct *vm_area_dup(struct vm_area_struct *);
 void vm_area_free(struct vm_area_struct *);
-/* Use only if VMA has no other users */
+/* Use only if VMA has no other users and might still be attached to a tree */
 void __vm_area_free(struct vm_area_struct *vma);
 
 #ifndef CONFIG_MMU
@@ -751,7 +751,7 @@  static inline void vma_init(struct vm_area_struct *vma, struct mm_struct *mm)
 	vma->vm_mm = mm;
 	vma->vm_ops = &dummy_vm_ops;
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
-	vma_mark_detached(vma, false);
+	vma->detached = true;
 	vma_numab_state_init(vma);
 }
 
diff --git a/kernel/fork.c b/kernel/fork.c
index 41c964104b58..000fc429345c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -540,6 +540,7 @@  static void vm_area_free_rcu_cb(struct rcu_head *head)
 
 	/* The vma should not be locked while being destroyed. */
 	VM_BUG_ON_VMA(rwsem_is_locked(&vma->vm_lock->lock), vma);
+	WARN_ON_ONCE(!vma->detached);
 	__vm_area_free(vma);
 }
 #endif
@@ -549,6 +550,7 @@  void vm_area_free(struct vm_area_struct *vma)
 #ifdef CONFIG_PER_VMA_LOCK
 	call_rcu(&vma->vm_rcu, vm_area_free_rcu_cb);
 #else
+	WARN_ON_ONCE(!vma->detached);
 	__vm_area_free(vma);
 #endif
 }
diff --git a/mm/internal.h b/mm/internal.h
index 68410c6d97ac..728189e6c703 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1068,6 +1068,7 @@  static inline void vma_iter_store(struct vma_iterator *vmi,
 	vmi->mas.index = vma->vm_start;
 	vmi->mas.last = vma->vm_end - 1;
 	mas_store_prealloc(&vmi->mas, vma);
+	vma_mark_detached(vma, false);
 }
 
 static inline int vma_iter_store_gfp(struct vma_iterator *vmi,