Bug 177821

Summary: NULL pointer dereference in list_rcu
Product: Memory Management Reporter: Alexander Polakov (apolyakov)
Component: OtherAssignee: Andrew Morton (akpm)
Status: NEW ---    
Severity: normal    
Priority: P1    
Hardware: All   
OS: Linux   
Kernel Version: 4.7.7 Subsystem:
Regression: No Bisected commit-id:
Attachments: Proposed fix

Description Alexander Polakov 2016-10-17 13:08:17 UTC
Created attachment 241871 [details]
Proposed fix

2016-10-15T10:26:10.704989+03:00 192.168.3.112 [395333.852698] CPU: 0 PID: 1006 Comm: vsftpd Not tainted 4.7.7-0-beget-acl #1
2016-10-15T10:26:10.705357+03:00 192.168.3.112 [395333.852904] Hardware name: Intel Corporation S5520UR/S5520UR, BIOS S5500.86B.01.00.0045.112320091052 11/23/2009
2016-10-15T10:26:10.705357+03:00 192.168.3.112  ffffffffa43733cb
2016-10-15T10:26:10.705367+03:00 192.168.3.112 [395333.853296]  0000000000000286
2016-10-15T10:26:10.705367+03:00 192.168.3.112  0000000000000000
2016-10-15T10:26:10.705567+03:00 192.168.3.112  ffff88086fc11050
2016-10-15T10:26:10.705765+03:00 192.168.3.112  
2016-10-15T10:26:10.705765+03:00 192.168.3.112 [395333.853690]  0000000000000004
2016-10-15T10:26:10.705765+03:00 192.168.3.112  0000000000000001
2016-10-15T10:26:10.705796+03:00 192.168.3.112  ffffffffa41704ed
2016-10-15T10:26:10.705968+03:00 192.168.3.112  ffff88082b259800
2016-10-15T10:26:10.706142+03:00 192.168.3.112  
2016-10-15T10:26:10.706178+03:00 192.168.3.112 [395333.854083]  024040c000000004
2016-10-15T10:26:10.706208+03:00 192.168.3.112  ffff88082c7d7b04
2016-10-15T10:26:10.706208+03:00 192.168.3.112  ffff88082c7d7ac0
2016-10-15T10:26:10.706376+03:00 192.168.3.112  00000000024040c0
2016-10-15T10:26:10.706535+03:00 192.168.3.112  
2016-10-15T10:26:10.706733+03:00 192.168.3.112 [395333.854474] Call Trace:
2016-10-15T10:26:10.706955+03:00 192.168.3.112 [395333.854668]  [<ffffffffa43733cb>] ? dump_stack+0x47/0x5c
2016-10-15T10:26:10.707151+03:00 192.168.3.112 [395333.854868]  [<ffffffffa41704ed>] ? warn_alloc_failed+0xfd/0x160
2016-10-15T10:26:10.707365+03:00 192.168.3.112 [395333.855071]  [<ffffffffa41e1d50>] ? __alloc_pages_direct_compact+0xf0/0x102
2016-10-15T10:26:10.707538+03:00 192.168.3.112 [395333.855278]  [<ffffffffa416e4d0>] ? drain_pages_zone+0x50/0x50
2016-10-15T10:26:10.707753+03:00 192.168.3.112 [395333.855483]  [<ffffffffa4170989>] ? __alloc_pages_nodemask+0x439/0xe70
2016-10-15T10:26:10.707979+03:00 192.168.3.112 [395333.855696]  [<ffffffffa40b72a7>] ? set_next_entity+0x177/0x640
2016-10-15T10:26:10.708203+03:00 192.168.3.112 [395333.855910]  [<ffffffffa41bc10a>] ? alloc_pages_current+0x9a/0x120
2016-10-15T10:26:10.708405+03:00 192.168.3.112 [395333.856122]  [<ffffffffa4171651>] ? alloc_kmem_pages+0x21/0xa0
2016-10-15T10:26:10.708605+03:00 192.168.3.112 [395333.856335]  [<ffffffffa418d1e8>] ? kmalloc_order+0x18/0x50
2016-10-15T10:26:10.708812+03:00 192.168.3.112 [395333.856544]  [<ffffffffa418d259>] ? kmalloc_order_trace+0x39/0xc0
2016-10-15T10:26:10.709029+03:00 192.168.3.112 [395333.856755]  [<ffffffffa4193d63>] ? __list_lru_init+0x1a3/0x210
2016-10-15T10:26:10.709230+03:00 192.168.3.112 [395333.856966]  [<ffffffffa4249050>] ? proc_fill_super+0xa0/0xa0
2016-10-15T10:26:10.709435+03:00 192.168.3.112 [395333.857173]  [<ffffffffa41e8ace>] ? sget+0x1be/0x3d0
2016-10-15T10:26:10.709651+03:00 192.168.3.112 [395333.857377]  [<ffffffffa4249440>] ? proc_mount+0x140/0x140
2016-10-15T10:26:10.709873+03:00 192.168.3.112 [395333.857591]  [<ffffffffa4249384>] ? proc_mount+0x84/0x140
2016-10-15T10:26:10.710076+03:00 192.168.3.112 [395333.857805]  [<ffffffffa41e9eba>] ? mount_fs+0x4a/0x190
2016-10-15T10:26:10.710320+03:00 192.168.3.112 [395333.858019]  [<ffffffffa42049c8>] ? vfs_kern_mount+0x78/0x130
2016-10-15T10:26:10.710503+03:00 192.168.3.112 [395333.858236]  [<ffffffffa4204a99>] ? kern_mount_data+0x19/0x40
2016-10-15T10:26:10.710721+03:00 192.168.3.112 [395333.858447]  [<ffffffffa4249528>] ? pid_ns_prepare_proc+0x18/0x30
2016-10-15T10:26:10.710939+03:00 192.168.3.112 [395333.858661]  [<ffffffffa40a45a6>] ? alloc_pid+0x436/0x470
2016-10-15T10:26:10.711152+03:00 192.168.3.112 [395333.858878]  [<ffffffffa40886bc>] ? copy_process+0xb7c/0x1d20
2016-10-15T10:26:10.711371+03:00 192.168.3.112 [395333.859096]  [<ffffffffa40899ae>] ? _do_fork+0x7e/0x320
2016-10-15T10:26:10.711612+03:00 192.168.3.112 [395333.859310]  [<ffffffffa408bcc0>] ? task_stopped_code+0x40/0x40
2016-10-15T10:26:10.711798+03:00 192.168.3.112 [395333.859525]  [<ffffffffa40028d3>] ? do_syscall_64+0x43/0x90
2016-10-15T10:26:10.712001+03:00 192.168.3.112 [395333.859736]  [<ffffffffa471abbc>] ? entry_SYSCALL64_slow_path+0x25/0x25
2016-10-15T10:26:10.712227+03:00 192.168.3.112 [395333.859960] Mem-Info:
2016-10-15T10:26:10.713535+03:00 192.168.3.112 [395333.860157] active_anon:2728884 inactive_anon:6664 isolated_anon:0
[395333.860157]  active_file:1496748 inactive_file:530865 isolated_file:0
[395333.860157]  unevictable:12979 dirty:467 writeback:0 unstable:0
[395333.860157]  slab_reclaimable:2360583 slab_unreclaimable:417834
[395333.860157]  mapped:31703 shmem:48061 pagetables:50936 bounce:0
[395333.860157]  free:508956 free_pcp:265 free_cma:11
2016-10-15T10:26:10.713545+03:00 192.168.3.112 [395333.861347] Node 0 
2016-10-15T10:26:10.714646+03:00 DMA free: 15888kB min:504kB low:628kB high:752kB active_anon:0kB inactive_anon:0kB active_file:0kB inactive_file:0kB unevictable:0kB isolated(anon):0kB isolated(file):0kB present:15972kB managed:15888kB mlocked:0kB dirty:0kB writeback:0kB mapped:0kB shmem:0kB slab_reclaimable:0kB slab_unreclaimable:0kB kernel_stack:0kB pagetables:0kB unstable:0kB bounce:0kB free_pcp:0kB local_pcp:0kB free_cma:0kB writeback_tmp:0kB pages_scanned:0 all_unreclaimable? yes



2016-10-15T10:26:10.723355+03:00 NULL pointer dereference
2016-10-15T10:26:10.723688+03:00 192.168.3.112  at 0000000000000008
2016-10-15T10:26:10.723697+03:00 192.168.3.112 [395333.871627] IP:
2016-10-15T10:26:10.723930+03:00 192.168.3.112  [<ffffffffa4193b00>] list_lru_destroy+0x40/0x100
2016-10-15T10:26:10.723930+03:00 192.168.3.112 [395333.871844] PGD 0 
2016-10-15T10:26:10.724098+03:00 192.168.3.112  
2016-10-15T10:26:10.731021+03:00 192.168.3.112 [395333.878747] CPU: 0 PID: 1006 Comm: vsftpd Not tainted 4.7.7-0-beget-acl #1
2016-10-15T10:26:10.731423+03:00 192.168.3.112 [395333.878958] Hardware name: Intel Corporation S5520UR/S5520UR, BIOS S5500.86B.01.00.0045.112320091052 11/23/2009
2016-10-15T10:26:10.731820+03:00 192.168.3.112 [395333.879361] task: ffff88082b259800 ti: ffff88082c7d4000 task.ti: ffff88082c7d4000
2016-10-15T10:26:10.731873+03:00 192.168.3.112 [395333.879758] RIP: 0010:[<ffffffffa4193b00>] 
2016-10-15T10:26:10.732311+03:00 192.168.3.112  [<ffffffffa4193b00>] list_lru_destroy+0x40/0x100
2016-10-15T10:26:10.732551+03:00 192.168.3.112 [395333.880164] RSP: 0018:ffff88082c7d7c08  EFLAGS: 00010246
2016-10-15T10:26:10.732824+03:00 192.168.3.112 [395333.880372] RAX: 0000000000000000 RBX: ffff880665cf0580 RCX: dead000000000200
2016-10-15T10:26:10.733211+03:00 192.168.3.112 [395333.880763] RDX: 0000000000000000 RSI: ffffea0016442300 RDI: ffffffffa4c5f880
2016-10-15T10:26:10.733620+03:00 192.168.3.112 [395333.881149] RBP: ffffffffa4c7bc20 R08: 0000000000000001 R09: ffff88059108e500
2016-10-15T10:26:10.733994+03:00 192.168.3.112 [395333.881540] R10: 000000000000001e R11: 0000000000001951 R12: ffff880665cf0000
2016-10-15T10:26:10.734422+03:00 192.168.3.112 [395333.881932] R13: fffffffffffffff4 R14: ffff8801837d70f0 R15: ffffffffa4c7bc58
2016-10-15T10:26:10.734772+03:00 192.168.3.112 [395333.882323] FS:  00007f6c8ea44700(0000) GS:ffff88086fc00000(0000) knlGS:0000000000000000
2016-10-15T10:26:10.734973+03:00 192.168.3.112 [395333.882707] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
2016-10-15T10:26:10.735355+03:00 192.168.3.112 [395333.882906] CR2: 0000000000000008 CR3: 000000082c59c000 CR4: 00000000000006f0
2016-10-15T10:26:10.735551+03:00 192.168.3.112 [395333.883291] Stack:
2016-10-15T10:26:10.735551+03:00 192.168.3.112 [395333.883479]  ffff880665cf0000
2016-10-15T10:26:10.735564+03:00 192.168.3.112  ffffffffa4c7bc20
2016-10-15T10:26:10.735564+03:00 192.168.3.112  ffff880665cf0000
2016-10-15T10:26:10.735758+03:00 192.168.3.112  ffffffffa41e8491
2016-10-15T10:26:10.735956+03:00 192.168.3.112 [395333.883873]  ffffffffa4249050
2016-10-15T10:26:10.735964+03:00 192.168.3.112  ffffffffa41e8c15
2016-10-15T10:26:10.735964+03:00 192.168.3.112  0000000000400000
2016-10-15T10:26:10.735948+03:00 192.168.3.112  
2016-10-15T10:26:10.736149+03:00 192.168.3.112  ffffffffa4249440
2016-10-15T10:26:10.736336+03:00 192.168.3.112  
2016-10-15T10:26:10.736336+03:00 192.168.3.112 [395333.884270]  0000000000000000
2016-10-15T10:26:10.736336+03:00 192.168.3.112  ffff8801837d70f0
2016-10-15T10:26:10.736377+03:00 192.168.3.112  0000000000000000
2016-10-15T10:26:10.736543+03:00 192.168.3.112  ffffffffa4c7bc20
2016-10-15T10:26:10.736726+03:00 192.168.3.112  
2016-10-15T10:26:10.736995+03:00 192.168.3.112 [395333.884666] Call Trace:
2016-10-15T10:26:10.737285+03:00 192.168.3.112 [395333.884864]  [<ffffffffa41e8491>] ? destroy_super+0x21/0x80
2016-10-15T10:26:10.737607+03:00 192.168.3.112 [395333.885070]  [<ffffffffa4249050>] ? proc_fill_super+0xa0/0xa0
2016-10-15T10:26:10.737840+03:00 192.168.3.112 [395333.885274]  [<ffffffffa41e8c15>] ? sget+0x305/0x3d0
2016-10-15T10:26:10.737984+03:00 192.168.3.112 [395333.885475]  [<ffffffffa4249440>] ? proc_mount+0x140/0x140
2016-10-15T10:26:10.737984+03:00 192.168.3.112 [395333.885677]  [<ffffffffa4249384>] ? proc_mount+0x84/0x140
2016-10-15T10:26:10.738137+03:00 192.168.3.112 [395333.885878]  [<ffffffffa41e9eba>] ? mount_fs+0x4a/0x190
2016-10-15T10:26:10.738345+03:00 192.168.3.112 [395333.886080]  [<ffffffffa42049c8>] ? vfs_kern_mount+0x78/0x130
2016-10-15T10:26:10.738562+03:00 192.168.3.112 [395333.886283]  [<ffffffffa4204a99>] ? kern_mount_data+0x19/0x40
2016-10-15T10:26:10.738760+03:00 192.168.3.112 [395333.886487]  [<ffffffffa4249528>] ? pid_ns_prepare_proc+0x18/0x30
2016-10-15T10:26:10.738955+03:00 192.168.3.112 [395333.886693]  [<ffffffffa40a45a6>] ? alloc_pid+0x436/0x470
2016-10-15T10:26:10.739164+03:00 192.168.3.112 [395333.886898]  [<ffffffffa40886bc>] ? copy_process+0xb7c/0x1d20
2016-10-15T10:26:10.739372+03:00 192.168.3.112 [395333.887104]  [<ffffffffa40899ae>] ? _do_fork+0x7e/0x320
2016-10-15T10:26:10.739577+03:00 192.168.3.112 [395333.887307]  [<ffffffffa408bcc0>] ? task_stopped_code+0x40/0x40
2016-10-15T10:26:10.739782+03:00 192.168.3.112 [395333.887515]  [<ffffffffa40028d3>] ? do_syscall_64+0x43/0x90
2016-10-15T10:26:10.740071+03:00 192.168.3.112 [395333.887720]  [<ffffffffa471abbc>] ? entry_SYSCALL64_slow_path+0x25/0x25
2016-10-15T10:26:10.740129+03:00 192.168.3.112 [395333.887926] Code: 
2016-10-15T10:26:10.741091+03:00 192.168.3.112 [395333.888846] RIP 
2016-10-15T10:26:10.741194+03:00 192.168.3.112  [<ffffffffa4193b00>] list_lru_destroy+0x40/0x100

After some analysis it seems to be that the problem is in alloc_super() function:
http://lxr.free-electrons.com/source/fs/super.c#L219
In case list_lru_init_memcg() fails it goes into destroy_super(), which calls list_lru_destroy().

And in list_lru_init() (http://lxr.free-electrons.com/source/mm/list_lru.c#L554) we see that in case memcg_init_list_lru() fails, lru->node is freed, but not set NULL, which then leads list_lru_destroy() to believe it is initialized and call memcg_destroy_list_lru(). memcg_destroy_list_lru() in turn can access lru->node[i].memcg_lrus, which is NULL.
Comment 1 Andrew Morton 2016-10-17 23:56:55 UTC
(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Mon, 17 Oct 2016 13:08:17 +0000 bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=177821
> 
>             Bug ID: 177821
>            Summary: NULL pointer dereference in list_rcu

Fair enough, I suppose.

Please don't submit patches via bugzilla - it is quite
painful.  Documentation/SubmittingPatches explains the
way to do it.

Here's what I put together.  Note that we do not have your
signed-off-by: for this.  Please send it?



From: Alexander Polakov <apolyakov@beget.ru>
Subject: mm/list_lru.c: avoid error-path NULL pointer deref

As described in https://bugzilla.kernel.org/show_bug.cgi?id=177821:

After some analysis it seems to be that the problem is in alloc_super(). 
In case list_lru_init_memcg() fails it goes into destroy_super(), which
calls list_lru_destroy().

And in list_lru_init() we see that in case memcg_init_list_lru() fails,
lru->node is freed, but not set NULL, which then leads list_lru_destroy()
to believe it is initialized and call memcg_destroy_list_lru(). 
memcg_destroy_list_lru() in turn can access lru->node[i].memcg_lrus, which
is NULL.

[akpm@linux-foundation.org: add comment]
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/list_lru.c |    2 ++
 1 file changed, 2 insertions(+)

diff -puN mm/list_lru.c~a mm/list_lru.c
--- a/mm/list_lru.c~a
+++ a/mm/list_lru.c
@@ -554,6 +554,8 @@ int __list_lru_init(struct list_lru *lru
 	err = memcg_init_list_lru(lru, memcg_aware);
 	if (err) {
 		kfree(lru->node);
+		/* Do this so a list_lru_destroy() doesn't crash: */
+		lru->node = NULL;
 		goto out;
 	}
 
_
Comment 2 Andrew Morton 2016-10-18 00:10:43 UTC
(resend due to "vdavydov@virtuozzo.com Unrouteable address")

(switched to email.  Please respond via emailed reply-to-all, not via the
bugzilla web interface).

On Mon, 17 Oct 2016 13:08:17 +0000 bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=177821
> 
>             Bug ID: 177821
>            Summary: NULL pointer dereference in list_rcu

Fair enough, I suppose.

Please don't submit patches via bugzilla - it is quite
painful.  Documentation/SubmittingPatches explains the
way to do it.

Here's what I put together.  Note that we do not have your
signed-off-by: for this.  Please send it?



From: Alexander Polakov <apolyakov@beget.ru>
Subject: mm/list_lru.c: avoid error-path NULL pointer deref

As described in https://bugzilla.kernel.org/show_bug.cgi?id=177821:

After some analysis it seems to be that the problem is in alloc_super(). 
In case list_lru_init_memcg() fails it goes into destroy_super(), which
calls list_lru_destroy().

And in list_lru_init() we see that in case memcg_init_list_lru() fails,
lru->node is freed, but not set NULL, which then leads list_lru_destroy()
to believe it is initialized and call memcg_destroy_list_lru(). 
memcg_destroy_list_lru() in turn can access lru->node[i].memcg_lrus, which
is NULL.

[akpm@linux-foundation.org: add comment]
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 mm/list_lru.c |    2 ++
 1 file changed, 2 insertions(+)

diff -puN mm/list_lru.c~a mm/list_lru.c
--- a/mm/list_lru.c~a
+++ a/mm/list_lru.c
@@ -554,6 +554,8 @@ int __list_lru_init(struct list_lru *lru
 	err = memcg_init_list_lru(lru, memcg_aware);
 	if (err) {
 		kfree(lru->node);
+		/* Do this so a list_lru_destroy() doesn't crash: */
+		lru->node = NULL;
 		goto out;
 	}
 
_
Comment 3 Alexander Polakov 2016-10-18 07:40:14 UTC
> On 18 Oct 2016, at 03:10, Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> 
> (resend due to "vdavydov@virtuozzo.com Unrouteable address")
> 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Mon, 17 Oct 2016 13:08:17 +0000 bugzilla-daemon@bugzilla.kernel.org wrote:
> 
>> https://bugzilla.kernel.org/show_bug.cgi?id=177821
>> 
>>            Bug ID: 177821
>>           Summary: NULL pointer dereference in list_rcu
> 
> Fair enough, I suppose.
> 
> Please don't submit patches via bugzilla - it is quite
> painful.  Documentation/SubmittingPatches explains the
> way to do it.
> 
> Here's what I put together.  Note that we do not have your
> signed-off-by: for this.  Please send it?

Sorry for the bugzilla thing, here's the patch with Signed-off-by added.
Hope I did it right.

From: Alexander Polakov <apolyakov@beget.ru>
Subject: mm/list_lru.c: avoid error-path NULL pointer deref

As described in https://bugzilla.kernel.org/show_bug.cgi?id=177821:

After some analysis it seems to be that the problem is in alloc_super(). 
In case list_lru_init_memcg() fails it goes into destroy_super(), which
calls list_lru_destroy().

And in list_lru_init() we see that in case memcg_init_list_lru() fails,
lru->node is freed, but not set NULL, which then leads list_lru_destroy()
to believe it is initialized and call memcg_destroy_list_lru(). 
memcg_destroy_list_lru() in turn can access lru->node[i].memcg_lrus, which
is NULL.

[akpm@linux-foundation.org: add comment]
Cc: Vladimir Davydov <vdavydov@parallels.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Alexander Polakov <apolyakov@beget.ru>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

mm/list_lru.c |    2 ++
1 file changed, 2 insertions(+)

diff -puN mm/list_lru.c~a mm/list_lru.c
--- a/mm/list_lru.c~a
+++ a/mm/list_lru.c
@@ -554,6 +554,8 @@ int __list_lru_init(struct list_lru *lru
	err = memcg_init_list_lru(lru, memcg_aware);
	if (err) {
		kfree(lru->node);
+		/* Do this so a list_lru_destroy() doesn't crash: */
+		lru->node = NULL;
		goto out;
	}

_


> 
> 
> 
> From: Alexander Polakov <apolyakov@beget.ru>
> Subject: mm/list_lru.c: avoid error-path NULL pointer deref
> 
> As described in https://bugzilla.kernel.org/show_bug.cgi?id=177821:
> 
> After some analysis it seems to be that the problem is in alloc_super(). 
> In case list_lru_init_memcg() fails it goes into destroy_super(), which
> calls list_lru_destroy().
> 
> And in list_lru_init() we see that in case memcg_init_list_lru() fails,
> lru->node is freed, but not set NULL, which then leads list_lru_destroy()
> to believe it is initialized and call memcg_destroy_list_lru(). 
> memcg_destroy_list_lru() in turn can access lru->node[i].memcg_lrus, which
> is NULL.
> 
> [akpm@linux-foundation.org: add comment]
> Cc: Vladimir Davydov <vdavydov@parallels.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
> 
> mm/list_lru.c |    2 ++
> 1 file changed, 2 insertions(+)
> 
> diff -puN mm/list_lru.c~a mm/list_lru.c
> --- a/mm/list_lru.c~a
> +++ a/mm/list_lru.c
> @@ -554,6 +554,8 @@ int __list_lru_init(struct list_lru *lru
>       err = memcg_init_list_lru(lru, memcg_aware);
>       if (err) {
>               kfree(lru->node);
> +             /* Do this so a list_lru_destroy() doesn't crash: */
> +             lru->node = NULL;
>               goto out;
>       }
> 
> _
> 
>