* [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it
@ 2023-06-20 23:57 Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 2/3] mm: change vma_start_read to fail to lock a detached VMA Suren Baghdasaryan
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Suren Baghdasaryan @ 2023-06-20 23:57 UTC (permalink / raw)
To: akpm
Cc: willy, torvalds, vegard.nossum, mpe, Liam.Howlett, lrh2000,
mgorman, linux-mm, linux-kernel, kernel-team, surenb
Current implementation of vma_start_read() checks VMA for being locked
before taking vma->vm_lock and then checks that again. This mechanism
fails to detect a case when the VMA gets write-locked, modified and
unlocked after the first check but before the vma->vm_lock is obtained.
While this is not strictly a problem (vma_start_read would not produce
a false unlocked result), this allows it to successfully lock a VMA which
got detached from the VMA tree while vma_start_read was locking it.
New condition checks for any change in vma->vm_lock_seq after we obtain
vma->vm_lock and will cause vma_start_read() to fail if the above race
occurs.
Fixes: 5e31275cc997 ("mm: add per-VMA lock and helper functions to control it")
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
include/linux/mm.h | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..8410da79c570 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -639,23 +639,24 @@ static inline void vma_numab_state_free(struct vm_area_struct *vma) {}
*/
static inline bool vma_start_read(struct vm_area_struct *vma)
{
- /* Check before locking. A race might cause false locked result. */
- if (vma->vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))
+ int vm_lock_seq = READ_ONCE(vma->vm_lock_seq);
+
+ /*
+ * Check if VMA is locked before taking vma->vm_lock. A race or
+ * mm_lock_seq overflow might cause false locked result.
+ */
+ if (vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))
return false;
if (unlikely(down_read_trylock(&vma->vm_lock->lock) == 0))
return false;
- /*
- * Overflow might produce false locked result.
- * False unlocked result is impossible because we modify and check
- * vma->vm_lock_seq under vma->vm_lock protection and mm->mm_lock_seq
- * modification invalidates all existing locks.
- */
- if (unlikely(vma->vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))) {
+ /* Fail if VMA was write-locked after we checked it earlier */
+ if (unlikely(vm_lock_seq != READ_ONCE(vma->vm_lock_seq))) {
up_read(&vma->vm_lock->lock);
return false;
}
+
return true;
}
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/3] mm: change vma_start_read to fail to lock a detached VMA
2023-06-20 23:57 [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
@ 2023-06-20 23:57 ` Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
2023-06-26 20:51 ` [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
2 siblings, 0 replies; 8+ messages in thread
From: Suren Baghdasaryan @ 2023-06-20 23:57 UTC (permalink / raw)
To: akpm
Cc: willy, torvalds, vegard.nossum, mpe, Liam.Howlett, lrh2000,
mgorman, linux-mm, linux-kernel, kernel-team, surenb
vma_start_read can successfully lock a detached VMA relying on its
caller to check for vma->detached. Change vma_start_read to include that
check and fail if the VMA got detached.
Suggested-by: Linus Torvalds <torvalds@linuxfoundation.org>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
include/linux/mm.h | 4 ++--
include/linux/vm_event_item.h | 1 -
mm/memory.c | 10 +---------
mm/vmstat.c | 1 -
4 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8410da79c570..74e3033c9fc2 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -651,8 +651,8 @@ static inline bool vma_start_read(struct vm_area_struct *vma)
if (unlikely(down_read_trylock(&vma->vm_lock->lock) == 0))
return false;
- /* Fail if VMA was write-locked after we checked it earlier */
- if (unlikely(vm_lock_seq != READ_ONCE(vma->vm_lock_seq))) {
+ /* Fail if VMA is detached or was write-locked after we checked it earlier */
+ if (unlikely(vma->detached || vm_lock_seq != READ_ONCE(vma->vm_lock_seq))) {
up_read(&vma->vm_lock->lock);
return false;
}
diff --git a/include/linux/vm_event_item.h b/include/linux/vm_event_item.h
index 8abfa1240040..e741098954ff 100644
--- a/include/linux/vm_event_item.h
+++ b/include/linux/vm_event_item.h
@@ -154,7 +154,6 @@ enum vm_event_item { PGPGIN, PGPGOUT, PSWPIN, PSWPOUT,
VMA_LOCK_SUCCESS,
VMA_LOCK_ABORT,
VMA_LOCK_RETRY,
- VMA_LOCK_MISS,
#endif
NR_VM_EVENT_ITEMS
};
diff --git a/mm/memory.c b/mm/memory.c
index f69fbc251198..bd666fa20748 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5275,7 +5275,7 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
struct vm_area_struct *vma;
rcu_read_lock();
-retry:
+
vma = mas_walk(&mas);
if (!vma)
goto inval;
@@ -5306,14 +5306,6 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm,
goto inval;
}
- /* Check if the VMA got isolated after we found it */
- if (vma->detached) {
- vma_end_read(vma);
- count_vm_vma_lock_event(VMA_LOCK_MISS);
- /* The area was replaced with another one */
- goto retry;
- }
-
rcu_read_unlock();
return vma;
inval:
diff --git a/mm/vmstat.c b/mm/vmstat.c
index c28046371b45..5d6acdb1b611 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1403,7 +1403,6 @@ const char * const vmstat_text[] = {
"vma_lock_success",
"vma_lock_abort",
"vma_lock_retry",
- "vma_lock_miss",
#endif
#endif /* CONFIG_VM_EVENT_COUNTERS || CONFIG_MEMCG */
};
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] mm: check for VMA being detached before destroying it
2023-06-20 23:57 [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 2/3] mm: change vma_start_read to fail to lock a detached VMA Suren Baghdasaryan
@ 2023-06-20 23:57 ` Suren Baghdasaryan
2023-06-21 0:05 ` Suren Baghdasaryan
` (2 more replies)
2023-06-26 20:51 ` [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
2 siblings, 3 replies; 8+ messages in thread
From: Suren Baghdasaryan @ 2023-06-20 23:57 UTC (permalink / raw)
To: akpm
Cc: willy, torvalds, vegard.nossum, mpe, Liam.Howlett, lrh2000,
mgorman, linux-mm, linux-kernel, kernel-team, surenb
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(-)
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,
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] mm: check for VMA being detached before destroying it
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
@ 2023-06-21 0:05 ` Suren Baghdasaryan
2023-06-21 2:15 ` kernel test robot
2023-06-21 5:53 ` kernel test robot
2 siblings, 0 replies; 8+ messages in thread
From: Suren Baghdasaryan @ 2023-06-21 0:05 UTC (permalink / raw)
To: akpm
Cc: willy, torvalds, vegard.nossum, mpe, Liam.Howlett, lrh2000,
mgorman, linux-mm, linux-kernel, kernel-team
On Tue, Jun 20, 2023 at 4:57 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> 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>
My tests did not generate the warning but the test coverage is far
from perfect, so if someone can run extensive testing on this one that
would be greatly appreciated.
Thanks,
Suren.
> ---
> include/linux/mm.h | 4 ++--
> kernel/fork.c | 2 ++
> mm/internal.h | 1 +
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> 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,
> --
> 2.41.0.162.gfafddb0af9-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] mm: check for VMA being detached before destroying it
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
2023-06-21 0:05 ` Suren Baghdasaryan
@ 2023-06-21 2:15 ` kernel test robot
2023-06-21 7:01 ` Suren Baghdasaryan
2023-06-21 5:53 ` kernel test robot
2 siblings, 1 reply; 8+ messages in thread
From: kernel test robot @ 2023-06-21 2:15 UTC (permalink / raw)
To: Suren Baghdasaryan, akpm
Cc: oe-kbuild-all, willy, torvalds, vegard.nossum, mpe, Liam.Howlett,
lrh2000, mgorman, linux-mm, linux-kernel, kernel-team, surenb
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] mm: check for VMA being detached before destroying it
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
2023-06-21 0:05 ` Suren Baghdasaryan
2023-06-21 2:15 ` kernel test robot
@ 2023-06-21 5:53 ` kernel test robot
2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2023-06-21 5:53 UTC (permalink / raw)
To: Suren Baghdasaryan, akpm
Cc: oe-kbuild-all, willy, torvalds, vegard.nossum, mpe, Liam.Howlett,
lrh2000, mgorman, linux-mm, linux-kernel, kernel-team, surenb
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
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 3/3] mm: check for VMA being detached before destroying it
2023-06-21 2:15 ` kernel test robot
@ 2023-06-21 7:01 ` Suren Baghdasaryan
0 siblings, 0 replies; 8+ messages in thread
From: Suren Baghdasaryan @ 2023-06-21 7:01 UTC (permalink / raw)
To: kernel test robot
Cc: akpm, oe-kbuild-all, willy, torvalds, vegard.nossum, mpe,
Liam.Howlett, lrh2000, mgorman, linux-mm, linux-kernel,
kernel-team
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it
2023-06-20 23:57 [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 2/3] mm: change vma_start_read to fail to lock a detached VMA Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
@ 2023-06-26 20:51 ` Suren Baghdasaryan
2 siblings, 0 replies; 8+ messages in thread
From: Suren Baghdasaryan @ 2023-06-26 20:51 UTC (permalink / raw)
To: akpm
Cc: willy, torvalds, vegard.nossum, mpe, Liam.Howlett, lrh2000,
mgorman, linux-mm, linux-kernel, kernel-team
On Tue, Jun 20, 2023 at 11:57 PM Suren Baghdasaryan <surenb@google.com> wrote:
>
> Current implementation of vma_start_read() checks VMA for being locked
> before taking vma->vm_lock and then checks that again. This mechanism
> fails to detect a case when the VMA gets write-locked, modified and
> unlocked after the first check but before the vma->vm_lock is obtained.
> While this is not strictly a problem (vma_start_read would not produce
> a false unlocked result), this allows it to successfully lock a VMA which
> got detached from the VMA tree while vma_start_read was locking it.
> New condition checks for any change in vma->vm_lock_seq after we obtain
> vma->vm_lock and will cause vma_start_read() to fail if the above race
> occurs.
Just a friendly ping for feedback.
I know most people were busy fixing the stack expansion problem and
these patches are not urgent, so no rush. If nobody reviews them, I'll
ping again next week.
>
> Fixes: 5e31275cc997 ("mm: add per-VMA lock and helper functions to control it")
> Suggested-by: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> ---
> include/linux/mm.h | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 27ce77080c79..8410da79c570 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -639,23 +639,24 @@ static inline void vma_numab_state_free(struct vm_area_struct *vma) {}
> */
> static inline bool vma_start_read(struct vm_area_struct *vma)
> {
> - /* Check before locking. A race might cause false locked result. */
> - if (vma->vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))
> + int vm_lock_seq = READ_ONCE(vma->vm_lock_seq);
> +
> + /*
> + * Check if VMA is locked before taking vma->vm_lock. A race or
> + * mm_lock_seq overflow might cause false locked result.
> + */
> + if (vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))
> return false;
>
> if (unlikely(down_read_trylock(&vma->vm_lock->lock) == 0))
> return false;
>
> - /*
> - * Overflow might produce false locked result.
> - * False unlocked result is impossible because we modify and check
> - * vma->vm_lock_seq under vma->vm_lock protection and mm->mm_lock_seq
> - * modification invalidates all existing locks.
> - */
> - if (unlikely(vma->vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))) {
> + /* Fail if VMA was write-locked after we checked it earlier */
> + if (unlikely(vm_lock_seq != READ_ONCE(vma->vm_lock_seq))) {
> up_read(&vma->vm_lock->lock);
> return false;
> }
> +
> return true;
> }
>
> --
> 2.41.0.162.gfafddb0af9-goog
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-06-26 20:51 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-20 23:57 [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 2/3] mm: change vma_start_read to fail to lock a detached VMA Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
2023-06-21 0:05 ` Suren Baghdasaryan
2023-06-21 2:15 ` kernel test robot
2023-06-21 7:01 ` Suren Baghdasaryan
2023-06-21 5:53 ` kernel test robot
2023-06-26 20:51 ` [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it Suren Baghdasaryan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).