Re: [PATCH] mm/vmstat: fix outdated vmstat_text

[Michal Hocko <mhocko@kernel.org>]

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