Skip to content

Commit ead91de

Browse files
name2965gregkh
authored andcommitted
mm/vmalloc: fix data race in show_numa_info()
commit 5c5f046 upstream. The following data-race was found in show_numa_info(): ================================================================== BUG: KCSAN: data-race in vmalloc_info_show / vmalloc_info_show read to 0xffff88800971fe30 of 4 bytes by task 8289 on cpu 0: show_numa_info mm/vmalloc.c:4936 [inline] vmalloc_info_show+0x5a8/0x7e0 mm/vmalloc.c:5016 seq_read_iter+0x373/0xb40 fs/seq_file.c:230 proc_reg_read_iter+0x11e/0x170 fs/proc/inode.c:299 .... write to 0xffff88800971fe30 of 4 bytes by task 8287 on cpu 1: show_numa_info mm/vmalloc.c:4934 [inline] vmalloc_info_show+0x38f/0x7e0 mm/vmalloc.c:5016 seq_read_iter+0x373/0xb40 fs/seq_file.c:230 proc_reg_read_iter+0x11e/0x170 fs/proc/inode.c:299 .... value changed: 0x0000008f -> 0x00000000 ================================================================== According to this report,there is a read/write data-race because m->private is accessible to multiple CPUs. To fix this, instead of allocating the heap in proc_vmalloc_init() and passing the heap address to m->private, vmalloc_info_show() should allocate the heap. Link: https://lkml.kernel.org/r/[email protected] Fixes: 8e1d743 ("mm: vmalloc: support multiple nodes in vmallocinfo") Signed-off-by: Jeongjun Park <[email protected]> Suggested-by: Eric Dumazet <[email protected]> Suggested-by: Andrew Morton <[email protected]> Reviewed-by: "Uladzislau Rezki (Sony)" <[email protected]> Signed-off-by: Andrew Morton <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 679bf9a commit ead91de

File tree

1 file changed

+35
-28
lines changed

1 file changed

+35
-28
lines changed

mm/vmalloc.c

Lines changed: 35 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3095,7 +3095,7 @@ static void clear_vm_uninitialized_flag(struct vm_struct *vm)
30953095
/*
30963096
* Before removing VM_UNINITIALIZED,
30973097
* we should make sure that vm has proper values.
3098-
* Pair with smp_rmb() in show_numa_info().
3098+
* Pair with smp_rmb() in vread_iter() and vmalloc_info_show().
30993099
*/
31003100
smp_wmb();
31013101
vm->flags &= ~VM_UNINITIALIZED;
@@ -4938,28 +4938,29 @@ bool vmalloc_dump_obj(void *object)
49384938
#endif
49394939

49404940
#ifdef CONFIG_PROC_FS
4941-
static void show_numa_info(struct seq_file *m, struct vm_struct *v)
4942-
{
4943-
if (IS_ENABLED(CONFIG_NUMA)) {
4944-
unsigned int nr, *counters = m->private;
4945-
unsigned int step = 1U << vm_area_page_order(v);
49464941

4947-
if (!counters)
4948-
return;
4942+
/*
4943+
* Print number of pages allocated on each memory node.
4944+
*
4945+
* This function can only be called if CONFIG_NUMA is enabled
4946+
* and VM_UNINITIALIZED bit in v->flags is disabled.
4947+
*/
4948+
static void show_numa_info(struct seq_file *m, struct vm_struct *v,
4949+
unsigned int *counters)
4950+
{
4951+
unsigned int nr;
4952+
unsigned int step = 1U << vm_area_page_order(v);
49494953

4950-
if (v->flags & VM_UNINITIALIZED)
4951-
return;
4952-
/* Pair with smp_wmb() in clear_vm_uninitialized_flag() */
4953-
smp_rmb();
4954+
if (!counters)
4955+
return;
49544956

4955-
memset(counters, 0, nr_node_ids * sizeof(unsigned int));
4957+
memset(counters, 0, nr_node_ids * sizeof(unsigned int));
49564958

4957-
for (nr = 0; nr < v->nr_pages; nr += step)
4958-
counters[page_to_nid(v->pages[nr])] += step;
4959-
for_each_node_state(nr, N_HIGH_MEMORY)
4960-
if (counters[nr])
4961-
seq_printf(m, " N%u=%u", nr, counters[nr]);
4962-
}
4959+
for (nr = 0; nr < v->nr_pages; nr += step)
4960+
counters[page_to_nid(v->pages[nr])] += step;
4961+
for_each_node_state(nr, N_HIGH_MEMORY)
4962+
if (counters[nr])
4963+
seq_printf(m, " N%u=%u", nr, counters[nr]);
49634964
}
49644965

49654966
static void show_purge_info(struct seq_file *m)
@@ -4987,6 +4988,10 @@ static int vmalloc_info_show(struct seq_file *m, void *p)
49874988
struct vmap_area *va;
49884989
struct vm_struct *v;
49894990
int i;
4991+
unsigned int *counters;
4992+
4993+
if (IS_ENABLED(CONFIG_NUMA))
4994+
counters = kmalloc(nr_node_ids * sizeof(unsigned int), GFP_KERNEL);
49904995

49914996
for (i = 0; i < nr_vmap_nodes; i++) {
49924997
vn = &vmap_nodes[i];
@@ -5003,6 +5008,11 @@ static int vmalloc_info_show(struct seq_file *m, void *p)
50035008
}
50045009

50055010
v = va->vm;
5011+
if (v->flags & VM_UNINITIALIZED)
5012+
continue;
5013+
5014+
/* Pair with smp_wmb() in clear_vm_uninitialized_flag() */
5015+
smp_rmb();
50065016

50075017
seq_printf(m, "0x%pK-0x%pK %7ld",
50085018
v->addr, v->addr + v->size, v->size);
@@ -5037,7 +5047,9 @@ static int vmalloc_info_show(struct seq_file *m, void *p)
50375047
if (is_vmalloc_addr(v->pages))
50385048
seq_puts(m, " vpages");
50395049

5040-
show_numa_info(m, v);
5050+
if (IS_ENABLED(CONFIG_NUMA))
5051+
show_numa_info(m, v, counters);
5052+
50415053
seq_putc(m, '\n');
50425054
}
50435055
spin_unlock(&vn->busy.lock);
@@ -5047,19 +5059,14 @@ static int vmalloc_info_show(struct seq_file *m, void *p)
50475059
* As a final step, dump "unpurged" areas.
50485060
*/
50495061
show_purge_info(m);
5062+
if (IS_ENABLED(CONFIG_NUMA))
5063+
kfree(counters);
50505064
return 0;
50515065
}
50525066

50535067
static int __init proc_vmalloc_init(void)
50545068
{
5055-
void *priv_data = NULL;
5056-
5057-
if (IS_ENABLED(CONFIG_NUMA))
5058-
priv_data = kmalloc(nr_node_ids * sizeof(unsigned int), GFP_KERNEL);
5059-
5060-
proc_create_single_data("vmallocinfo",
5061-
0400, NULL, vmalloc_info_show, priv_data);
5062-
5069+
proc_create_single("vmallocinfo", 0400, NULL, vmalloc_info_show);
50635070
return 0;
50645071
}
50655072
module_init(proc_vmalloc_init);

0 commit comments

Comments
 (0)