* [PATCH] mm/vmstat: fix outdated vmstat_text
@ 2018-09-29 1:36 Jann Horn
2018-09-29 3:07 ` kbuild test robot
2018-10-03 16:29 ` Michal Hocko
0 siblings, 2 replies; 5+ messages in thread
From: Jann Horn @ 2018-09-29 1:36 UTC (permalink / raw)
To: Andrew Morton, linux-mm, jannh
Cc: Davidlohr Bueso, Oleg Nesterov, Linus Torvalds,
Christoph Lameter, Roman Gushchin, Kemi Wang, Kees Cook
commit 7a9cdebdcc17 ("mm: get rid of vmacache_flush_all() entirely")
removed the VMACACHE_FULL_FLUSHES statistics, but didn't remove the
corresponding entry in vmstat_text. This causes an out-of-bounds access in
vmstat_show().
Luckily this only affects kernels with CONFIG_DEBUG_VM_VMACACHE=y, which is
probably very rare.
Having two gigantic arrays that must be kept in sync isn't exactly robust.
To make it easier to catch such issues in the future, add a BUILD_BUG_ON().
Fixes: 7a9cdebdcc17 ("mm: get rid of vmacache_flush_all() entirely")
Cc: stable@vger.kernel.org
Signed-off-by: Jann Horn <jannh@google.com>
---
mm/vmstat.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/mm/vmstat.c b/mm/vmstat.c
index 8ba0870ecddd..db6379a3f8bf 100644
--- a/mm/vmstat.c
+++ b/mm/vmstat.c
@@ -1283,7 +1283,6 @@ const char * const vmstat_text[] = {
#ifdef CONFIG_DEBUG_VM_VMACACHE
"vmacache_find_calls",
"vmacache_find_hits",
- "vmacache_full_flushes",
#endif
#ifdef CONFIG_SWAP
"swap_ra",
@@ -1661,6 +1660,8 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos)
stat_items_size += sizeof(struct vm_event_state);
#endif
+ BUILD_BUG_ON(stat_items_size !=
+ ARRAY_SIZE(vmstat_text) * sizeof(unsigned long));
v = kmalloc(stat_items_size, GFP_KERNEL);
m->private = v;
if (!v)
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmstat: fix outdated vmstat_text
2018-09-29 1:36 [PATCH] mm/vmstat: fix outdated vmstat_text Jann Horn
@ 2018-09-29 3:07 ` kbuild test robot
2018-10-01 13:46 ` Jann Horn
2018-10-03 16:29 ` Michal Hocko
1 sibling, 1 reply; 5+ messages in thread
From: kbuild test robot @ 2018-09-29 3:07 UTC (permalink / raw)
To: Jann Horn
Cc: kbuild-all, Andrew Morton, linux-mm, Davidlohr Bueso,
Oleg Nesterov, Linus Torvalds, Christoph Lameter, Roman Gushchin,
Kemi Wang, Kees Cook
[-- Attachment #1: Type: text/plain, Size: 3742 bytes --]
Hi Jann,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc5 next-20180928]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jann-Horn/mm-vmstat-fix-outdated-vmstat_text/20180929-102147
config: i386-randconfig-x005-201838 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All error/warnings (new ones prefixed by >>):
In file included from include/linux/export.h:45:0,
from include/linux/linkage.h:7,
from include/linux/fs.h:5,
from mm/vmstat.c:12:
mm/vmstat.c: In function 'vmstat_start':
>> include/linux/compiler.h:358:38: error: call to '__compiletime_assert_1664' declared with attribute error: BUILD_BUG_ON failed: stat_items_size != ARRAY_SIZE(vmstat_text) * sizeof(unsigned long)
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:338:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:358:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:45:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:69:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
^~~~~~~~~~~~~~~~
>> mm/vmstat.c:1663:2: note: in expansion of macro 'BUILD_BUG_ON'
BUILD_BUG_ON(stat_items_size !=
^~~~~~~~~~~~
vim +/__compiletime_assert_1664 +358 include/linux/compiler.h
9a8ab1c3 Daniel Santos 2013-02-21 344
9a8ab1c3 Daniel Santos 2013-02-21 345 #define _compiletime_assert(condition, msg, prefix, suffix) \
9a8ab1c3 Daniel Santos 2013-02-21 346 __compiletime_assert(condition, msg, prefix, suffix)
9a8ab1c3 Daniel Santos 2013-02-21 347
9a8ab1c3 Daniel Santos 2013-02-21 348 /**
9a8ab1c3 Daniel Santos 2013-02-21 349 * compiletime_assert - break build and emit msg if condition is false
9a8ab1c3 Daniel Santos 2013-02-21 350 * @condition: a compile-time constant condition to check
9a8ab1c3 Daniel Santos 2013-02-21 351 * @msg: a message to emit if condition is false
9a8ab1c3 Daniel Santos 2013-02-21 352 *
9a8ab1c3 Daniel Santos 2013-02-21 353 * In tradition of POSIX assert, this macro will break the build if the
9a8ab1c3 Daniel Santos 2013-02-21 354 * supplied condition is *false*, emitting the supplied error message if the
9a8ab1c3 Daniel Santos 2013-02-21 355 * compiler has support to do so.
9a8ab1c3 Daniel Santos 2013-02-21 356 */
9a8ab1c3 Daniel Santos 2013-02-21 357 #define compiletime_assert(condition, msg) \
9a8ab1c3 Daniel Santos 2013-02-21 @358 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
9a8ab1c3 Daniel Santos 2013-02-21 359
:::::: The code at line 358 was first introduced by commit
:::::: 9a8ab1c39970a4938a72d94e6fd13be88a797590 bug.h, compiler.h: introduce compiletime_assert & BUILD_BUG_ON_MSG
:::::: TO: Daniel Santos <daniel.santos@pobox.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36100 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmstat: fix outdated vmstat_text
2018-09-29 3:07 ` kbuild test robot
@ 2018-10-01 13:46 ` Jann Horn
0 siblings, 0 replies; 5+ messages in thread
From: Jann Horn @ 2018-10-01 13:46 UTC (permalink / raw)
To: lkp
Cc: kbuild-all, Andrew Morton, Linux-MM, Davidlohr Bueso,
Oleg Nesterov, Linus Torvalds, guro, kemi.wang, Kees Cook,
Christopher Lameter
On Sat, Sep 29, 2018 at 5:07 AM kbuild test robot <lkp@intel.com> wrote:
>
> Hi Jann,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v4.19-rc5 next-20180928]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url: https://github.com/0day-ci/linux/commits/Jann-Horn/mm-vmstat-fix-outdated-vmstat_text/20180929-102147
> config: i386-randconfig-x005-201838 (attached as .config)
> compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> All error/warnings (new ones prefixed by >>):
>
> In file included from include/linux/export.h:45:0,
> from include/linux/linkage.h:7,
> from include/linux/fs.h:5,
> from mm/vmstat.c:12:
> mm/vmstat.c: In function 'vmstat_start':
> >> include/linux/compiler.h:358:38: error: call to '__compiletime_assert_1664' declared with attribute error: BUILD_BUG_ON failed: stat_items_size != ARRAY_SIZE(vmstat_text) * sizeof(unsigned long)
Nice. Looks like the 0day test bot indeed found another mismatch:
#ifdef CONFIG_DEBUG_TLBFLUSH
#ifdef CONFIG_SMP
"nr_tlb_remote_flush",
"nr_tlb_remote_flush_received",
#endif /* CONFIG_SMP */
"nr_tlb_local_flush_all",
"nr_tlb_local_flush_one",
#endif /* CONFIG_DEBUG_TLBFLUSH */
vs
#ifdef CONFIG_DEBUG_TLBFLUSH
NR_TLB_REMOTE_FLUSH, /* cpu tried to flush others' tlbs */
NR_TLB_REMOTE_FLUSH_RECEIVED,/* cpu received ipi for flush */
NR_TLB_LOCAL_FLUSH_ALL,
NR_TLB_LOCAL_FLUSH_ONE,
#endif /* CONFIG_DEBUG_TLBFLUSH */
So if you build with CONFIG_VM_EVENT_COUNTERS=y &&
CONFIG_DEBUG_TLBFLUSH=y && CONFIG_SMP=n, vmstat output is bogus.
I like having my decisions to add asserts immediately validated by the
0day test bot. ^^
I'll send a v2 with that fixed up.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmstat: fix outdated vmstat_text
2018-09-29 1:36 [PATCH] mm/vmstat: fix outdated vmstat_text Jann Horn
2018-09-29 3:07 ` kbuild test robot
@ 2018-10-03 16:29 ` Michal Hocko
2018-10-03 16:31 ` Jann Horn
1 sibling, 1 reply; 5+ messages in thread
From: Michal Hocko @ 2018-10-03 16:29 UTC (permalink / raw)
To: Jann Horn
Cc: Andrew Morton, linux-mm, Davidlohr Bueso, Oleg Nesterov,
Linus Torvalds, Christoph Lameter, Roman Gushchin, Kemi Wang,
Kees Cook
On Sat 29-09-18 03:36:11, Jann Horn wrote:
> commit 7a9cdebdcc17 ("mm: get rid of vmacache_flush_all() entirely")
> removed the VMACACHE_FULL_FLUSHES statistics, but didn't remove the
> corresponding entry in vmstat_text. This causes an out-of-bounds access in
> vmstat_show().
>
> Luckily this only affects kernels with CONFIG_DEBUG_VM_VMACACHE=y, which is
> probably very rare.
>
> Having two gigantic arrays that must be kept in sync isn't exactly robust.
> To make it easier to catch such issues in the future, add a BUILD_BUG_ON().
>
> Fixes: 7a9cdebdcc17 ("mm: get rid of vmacache_flush_all() entirely")
> Cc: stable@vger.kernel.org
> Signed-off-by: Jann Horn <jannh@google.com>
Those could be two separate patches but anyway
Acked-by: Michal Hocko <mhocko@suse.com>
to both changes. I have burned myself on this in the past as well. Build
bugon would save me a lot of debugging.
> ---
> mm/vmstat.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/mm/vmstat.c b/mm/vmstat.c
> index 8ba0870ecddd..db6379a3f8bf 100644
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1283,7 +1283,6 @@ const char * const vmstat_text[] = {
> #ifdef CONFIG_DEBUG_VM_VMACACHE
> "vmacache_find_calls",
> "vmacache_find_hits",
> - "vmacache_full_flushes",
> #endif
> #ifdef CONFIG_SWAP
> "swap_ra",
> @@ -1661,6 +1660,8 @@ static void *vmstat_start(struct seq_file *m, loff_t *pos)
> stat_items_size += sizeof(struct vm_event_state);
> #endif
>
> + BUILD_BUG_ON(stat_items_size !=
> + ARRAY_SIZE(vmstat_text) * sizeof(unsigned long));
> v = kmalloc(stat_items_size, GFP_KERNEL);
> m->private = v;
> if (!v)
> --
> 2.19.0.605.g01d371f741-goog
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] mm/vmstat: fix outdated vmstat_text
2018-10-03 16:29 ` Michal Hocko
@ 2018-10-03 16:31 ` Jann Horn
0 siblings, 0 replies; 5+ messages in thread
From: Jann Horn @ 2018-10-03 16:31 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, Linux-MM, Davidlohr Bueso, Oleg Nesterov,
Linus Torvalds, clameter, guro, kemi.wang, Kees Cook
On Wed, Oct 3, 2018 at 6:29 PM Michal Hocko <mhocko@kernel.org> wrote:
>
> On Sat 29-09-18 03:36:11, Jann Horn wrote:
> > commit 7a9cdebdcc17 ("mm: get rid of vmacache_flush_all() entirely")
> > removed the VMACACHE_FULL_FLUSHES statistics, but didn't remove the
> > corresponding entry in vmstat_text. This causes an out-of-bounds access in
> > vmstat_show().
> >
> > Luckily this only affects kernels with CONFIG_DEBUG_VM_VMACACHE=y, which is
> > probably very rare.
> >
> > Having two gigantic arrays that must be kept in sync isn't exactly robust.
> > To make it easier to catch such issues in the future, add a BUILD_BUG_ON().
> >
> > Fixes: 7a9cdebdcc17 ("mm: get rid of vmacache_flush_all() entirely")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jann Horn <jannh@google.com>
>
> Those could be two separate patches but anyway
> Acked-by: Michal Hocko <mhocko@suse.com>
>
> to both changes. I have burned myself on this in the past as well. Build
> bugon would save me a lot of debugging.
I actually sent a v2 that splits this into two patches, and adds
another fix for nr_tlb_remote_flush and nr_tlb_remote_flush_received
for systems with CONFIG_VM_EVENT_COUNTERS=y && CONFIG_DEBUG_TLBFLUSH=y
&& CONFIG_SMP=n. akpm has already added the v2 patches to the mm tree.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-10-03 16:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-29 1:36 [PATCH] mm/vmstat: fix outdated vmstat_text Jann Horn
2018-09-29 3:07 ` kbuild test robot
2018-10-01 13:46 ` Jann Horn
2018-10-03 16:29 ` Michal Hocko
2018-10-03 16:31 ` Jann Horn
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.