* Re: [PATCH 3/3] mm: check for VMA being detached before destroying it
@ 2023-06-21 7:07 kernel test robot
0 siblings, 0 replies; 6+ messages in thread
From: kernel test robot @ 2023-06-21 7:07 UTC (permalink / raw)
To: oe-kbuild; +Cc: lkp
::::::
:::::: Manual check reason: "um arch report"
::::::
BCC: lkp@intel.com
CC: llvm@lists.linux.dev
CC: oe-kbuild-all@lists.linux.dev
In-Reply-To: <20230620235726.3873043-3-surenb@google.com>
References: <20230620235726.3873043-3-surenb@google.com>
TO: Suren Baghdasaryan <surenb@google.com>
TO: akpm@linux-foundation.org
CC: willy@infradead.org
CC: torvalds@linuxfoundation.org
CC: vegard.nossum@oracle.com
CC: mpe@ellerman.id.au
CC: Liam.Howlett@oracle.com
CC: lrh2000@pku.edu.cn
CC: mgorman@suse.de
CC: linux-mm@kvack.org
CC: linux-kernel@vger.kernel.org
CC: kernel-team@android.com
CC: surenb@google.com
Hi Suren,
kernel test robot noticed the following build warnings:
[auto build test WARNING 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
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago
config: um-randconfig-r012-20230620 (https://download.01.org/0day-ci/archive/20230621/202306211405.7zLvlu5T-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce: (https://download.01.org/0day-ci/archive/20230621/202306211405.7zLvlu5T-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/r/202306211405.7zLvlu5T-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:241:10: warning: array index 1 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
241 | case 2: set->sig[1] = 0;
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from net/ipv4/fib_trie.c:43:
In file included from include/linux/mm.h:989:
In file included from include/linux/huge_mm.h:8:
In file included from include/linux/fs.h:33:
In file included from include/linux/percpu-rwsem.h:7:
In file included from include/linux/rcuwait.h:6:
In file included from include/linux/sched/signal.h:6:
include/linux/signal.h:254:10: warning: array index 1 is past the end of the array (that has type 'unsigned long[1]') [-Warray-bounds]
254 | case 2: set->sig[1] = -1;
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
In file included from net/ipv4/fib_trie.c:49:
In file included from include/linux/inet.h:42:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from net/ipv4/fib_trie.c:49:
In file included from include/linux/inet.h:42:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from net/ipv4/fib_trie.c:49:
In file included from include/linux/inet.h:42:
In file included from include/net/net_namespace.h:43:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
>> net/ipv4/fib_trie.c:325:13: warning: implicit conversion from 'unsigned long' to 'u32' (aka 'unsigned int') changes value from 2305843009213693946 to 4294967290 [-Wconstant-conversion]
325 | if (bits > TNODE_VMALLOC_MAX)
| ^~~~~~~~~~~~~~~~~
net/ipv4/fib_trie.c:306:35: note: expanded from macro 'TNODE_VMALLOC_MAX'
306 | ilog2((SIZE_MAX - TNODE_SIZE(0)) / sizeof(struct key_vector *))
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/log2.h:162:14: note: expanded from macro 'ilog2'
162 | __ilog2_u32(n) : \
| ~~~~~~~~~~~ ^
62 warnings and 1 error generated.
--
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:8:
include/linux/mm.h:753:7: error: no member named 'detached' in 'struct vm_area_struct'
753 | vma->detached = true;
| ~~~ ^
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
547 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
560 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:573:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
573 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from drivers/tty/tty_port.c:8:
In file included from include/linux/tty.h:12:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:42:
In file included from include/linux/scatterlist.h:9:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:584:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
584 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:594:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
594 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:604:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
604 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:692:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
692 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:700:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
700 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:708:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
708 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:717:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
717 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:726:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
726 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:735:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
735 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
>> drivers/tty/tty_port.c:266:2: warning: implicit conversion from 'unsigned long' to 'unsigned int' changes value from 18446744073709551615 to 4294967295 [-Wconstant-conversion]
266 | INIT_KFIFO(port->xmit_fifo);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/kfifo.h:130:69: note: expanded from macro 'INIT_KFIFO'
130 | __kfifo->mask = __is_kfifo_ptr(__tmp) ? 0 : ARRAY_SIZE(__tmp->buf) - 1;\
| ~ ~~~~~~~~~~~~~~~~~~~~~~~^~~
13 warnings and 1 error generated.
vim +325 net/ipv4/fib_trie.c
37fd30f2da573c Alexander Duyck 2014-12-31 319
dc35dbeda3e00a Alexander Duyck 2015-03-06 320 static struct tnode *tnode_alloc(int bits)
f0e36f8cee8101 Patrick McHardy 2005-07-05 321 {
1de3d87bcd2c2e Alexander Duyck 2015-03-04 322 size_t size;
1de3d87bcd2c2e Alexander Duyck 2015-03-04 323
1de3d87bcd2c2e Alexander Duyck 2015-03-04 324 /* verify bits is within bounds */
1de3d87bcd2c2e Alexander Duyck 2015-03-04 @325 if (bits > TNODE_VMALLOC_MAX)
1de3d87bcd2c2e Alexander Duyck 2015-03-04 326 return NULL;
1de3d87bcd2c2e Alexander Duyck 2015-03-04 327
1de3d87bcd2c2e Alexander Duyck 2015-03-04 328 /* determine size and verify it is non-zero and didn't overflow */
1de3d87bcd2c2e Alexander Duyck 2015-03-04 329 size = TNODE_SIZE(1ul << bits);
1de3d87bcd2c2e Alexander Duyck 2015-03-04 330
2373ce1ca04dd4 Robert Olsson 2005-08-25 331 if (size <= PAGE_SIZE)
8d96544475b236 Eric Dumazet 2008-01-13 332 return kzalloc(size, GFP_KERNEL);
15be75cdb5db44 Stephen Hemminger 2008-04-10 333 else
7a1c8e5ab120a5 Eric Dumazet 2010-11-20 334 return vzalloc(size);
15be75cdb5db44 Stephen Hemminger 2008-04-10 335 }
2373ce1ca04dd4 Robert Olsson 2005-08-25 336
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 6+ messages in thread* [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 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
0 siblings, 1 reply; 6+ 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] 6+ 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 ` Suren Baghdasaryan
2023-06-21 0:05 ` Suren Baghdasaryan
` (2 more replies)
0 siblings, 3 replies; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ 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; 6+ 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] 6+ messages in thread
end of thread, other threads:[~2023-06-21 7:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-21 7:07 [PATCH 3/3] mm: check for VMA being detached before destroying it kernel test robot
-- strict thread matches above, loose matches on Subject: below --
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 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
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.