[resend PATCH 0/2] fix VFS register_shrinker fixup

[resend PATCH 0/2] fix VFS register_shrinker fixup

[Michal Hocko]

Hi Andrew,
Tetsuo has posted patch 1 already [1]. I had some minor concenrs about
the changelog but the approach was already OK. Aliaksei came with an
alternative patch [2] which also handles double unregistration. I have
updated the changelog and moved the syzbot report to the 2nd patch
because it is more related to the change there. The patch 1 is
prerequisite. Maybe we should just merge those two. I've kept Tetsuo's
s-o-b and his original authorship, but let me know if you disagree with
the new wording or the additional change, Tetsuo.

The patch 2 is based on Al's suggestion [3] and it fixes sget_userns
shrinker registration code.

Both of these stalled so can we have them merged finally?

[1] http://lkml.kernel.org/r/1511523385-6433-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
[2] http://lkml.kernel.org/r/20171216192937.13549-1-akaraliou.dev@gmail.com
[3] http://lkml.kernel.org/r/20171123145540.GB21978@ZenIV.linux.org.uk

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 1/2] mm,vmscan: Make unregister_shrinker() no-op if register_shrinker() failed.

[Michal Hocko]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

Since allowing register_shrinker() callers to call unregister_shrinker()
when register_shrinker() failed can simplify error recovery path, this
patch makes unregister_shrinker() no-op when register_shrinker() failed.
Let's also make sure that double unregister_shrinker doesn't blow up as
well and NULL nr_deferred on successful de-registration to make the
clean up even simpler and prevent from potential memory corruptions.

[akaraliou.dev@gmail.com: set nr_deferred = NULL to handle double
 unregister]
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Glauber Costa <glauber@scylladb.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 mm/vmscan.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mm/vmscan.c b/mm/vmscan.c
index 80dea50f421b..7a5801040fd4 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -281,10 +281,13 @@ EXPORT_SYMBOL(register_shrinker);
  */
 void unregister_shrinker(struct shrinker *shrinker)
 {
+	if (!shrinker->nr_deferred)
+		return;
 	down_write(&shrinker_rwsem);
 	list_del(&shrinker->list);
 	up_write(&shrinker_rwsem);
 	kfree(shrinker->nr_deferred);
+	shrinker->nr_deferred = NULL;
 }
 EXPORT_SYMBOL(unregister_shrinker);
 
-- 
2.15.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH 2/2] VFS: handle register_shrinker failure in sget_userns

[Michal Hocko]

From: Michal Hocko <mhocko@suse.com>

Syzbot caught an oops at unregister_shrinker() because combination of
commit 1d3d4437eae1bb29 ("vmscan: per-node deferred work") and fault
injection made register_shrinker() fail and the caller of
register_shrinker() did not check for failure.

----------
[  554.881422] FAULT_INJECTION: forcing a failure.
[  554.881422] name failslab, interval 1, probability 0, space 0, times 0
[  554.881438] CPU: 1 PID: 13231 Comm: syz-executor1 Not tainted 4.14.0-rc8+ #82
[  554.881443] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[  554.881445] Call Trace:
[  554.881459]  dump_stack+0x194/0x257
[  554.881474]  ? arch_local_irq_restore+0x53/0x53
[  554.881486]  ? find_held_lock+0x35/0x1d0
[  554.881507]  should_fail+0x8c0/0xa40
[  554.881522]  ? fault_create_debugfs_attr+0x1f0/0x1f0
[  554.881537]  ? check_noncircular+0x20/0x20
[  554.881546]  ? find_next_zero_bit+0x2c/0x40
[  554.881560]  ? ida_get_new_above+0x421/0x9d0
[  554.881577]  ? find_held_lock+0x35/0x1d0
[  554.881594]  ? __lock_is_held+0xb6/0x140
[  554.881628]  ? check_same_owner+0x320/0x320
[  554.881634]  ? lock_downgrade+0x990/0x990
[  554.881649]  ? find_held_lock+0x35/0x1d0
[  554.881672]  should_failslab+0xec/0x120
[  554.881684]  __kmalloc+0x63/0x760
[  554.881692]  ? lock_downgrade+0x990/0x990
[  554.881712]  ? register_shrinker+0x10e/0x2d0
[  554.881721]  ? trace_event_raw_event_module_request+0x320/0x320
[  554.881737]  register_shrinker+0x10e/0x2d0
[  554.881747]  ? prepare_kswapd_sleep+0x1f0/0x1f0
[  554.881755]  ? _down_write_nest_lock+0x120/0x120
[  554.881765]  ? memcpy+0x45/0x50
[  554.881785]  sget_userns+0xbcd/0xe20
(...snipped...)
[  554.898693] kasan: CONFIG_KASAN_INLINE enabled
[  554.898724] kasan: GPF could be caused by NULL-ptr deref or user memory access
[  554.898732] general protection fault: 0000 [#1] SMP KASAN
[  554.898737] Dumping ftrace buffer:
[  554.898741]    (ftrace buffer empty)
[  554.898743] Modules linked in:
[  554.898752] CPU: 1 PID: 13231 Comm: syz-executor1 Not tainted 4.14.0-rc8+ #82
[  554.898755] Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[  554.898760] task: ffff8801d1dbe5c0 task.stack: ffff8801c9e38000
[  554.898772] RIP: 0010:__list_del_entry_valid+0x7e/0x150
[  554.898775] RSP: 0018:ffff8801c9e3f108 EFLAGS: 00010246
[  554.898780] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
[  554.898784] RDX: 0000000000000000 RSI: ffff8801c53c6f98 RDI: ffff8801c53c6fa0
[  554.898788] RBP: ffff8801c9e3f120 R08: 1ffff100393c7d55 R09: 0000000000000004
[  554.898791] R10: ffff8801c9e3ef70 R11: 0000000000000000 R12: 0000000000000000
[  554.898795] R13: dffffc0000000000 R14: 1ffff100393c7e45 R15: ffff8801c53c6f98
[  554.898800] FS:  0000000000000000(0000) GS:ffff8801db300000(0000) knlGS:0000000000000000
[  554.898804] CS:  0010 DS: 002b ES: 002b CR0: 0000000080050033
[  554.898807] CR2: 00000000dbc23000 CR3: 00000001c7269000 CR4: 00000000001406e0
[  554.898813] DR0: 0000000020000000 DR1: 0000000020000000 DR2: 0000000000000000
[  554.898816] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000600
[  554.898818] Call Trace:
[  554.898828]  unregister_shrinker+0x79/0x300
[  554.898837]  ? perf_trace_mm_vmscan_writepage+0x750/0x750
[  554.898844]  ? down_write+0x87/0x120
[  554.898851]  ? deactivate_super+0x139/0x1b0
[  554.898857]  ? down_read+0x150/0x150
[  554.898864]  ? check_same_owner+0x320/0x320
[  554.898875]  deactivate_locked_super+0x64/0xd0
[  554.898883]  deactivate_super+0x141/0x1b0
----------

The oops has been already fixed by "mm,vmscan: Make
unregister_shrinker() no-op if register_shrinker() failed" but we still
want to fail the whole sget_userns because a fs without registered
shrinker could DoS the system easily.

Reported-by: syzbot <syzkaller@googlegroups.com>
Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Michal Hocko <mhocko@suse.com>
---
 fs/super.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 994db21f59bf..1b4c88e2ce9e 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -522,7 +522,10 @@ struct super_block *sget_userns(struct file_system_type *type,
 	hlist_add_head(&s->s_instances, &type->fs_supers);
 	spin_unlock(&sb_lock);
 	get_filesystem(type);
-	register_shrinker(&s->s_shrink);
+	if (unlikely(register_shrinker(&s->s_shrink) != 0)) {
+		deactivate_locked_super(s);
+		s = ERR_PTR(-ENOMEM);
+	}
 	return s;
 }
 
-- 
2.15.1

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [resend PATCH 0/2] fix VFS register_shrinker fixup

[Michal Hocko]

Dohh, I have missed resend by Tetsuo http://lkml.kernel.org/r/1513596701-4518-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
(thanks for dropping me from the CC).  Al seeemed to take the patch. We
still need patch 2. Al, are you going to take it from this thread or you
are going to go your way?

On Tue 19-12-17 14:28:42, Michal Hocko wrote:
> Hi Andrew,
> Tetsuo has posted patch 1 already [1]. I had some minor concenrs about
> the changelog but the approach was already OK. Aliaksei came with an
> alternative patch [2] which also handles double unregistration. I have
> updated the changelog and moved the syzbot report to the 2nd patch
> because it is more related to the change there. The patch 1 is
> prerequisite. Maybe we should just merge those two. I've kept Tetsuo's
> s-o-b and his original authorship, but let me know if you disagree with
> the new wording or the additional change, Tetsuo.
> 
> The patch 2 is based on Al's suggestion [3] and it fixes sget_userns
> shrinker registration code.
> 
> Both of these stalled so can we have them merged finally?
> 
> [1] http://lkml.kernel.org/r/1511523385-6433-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
> [2] http://lkml.kernel.org/r/20171216192937.13549-1-akaraliou.dev@gmail.com
> [3] http://lkml.kernel.org/r/20171123145540.GB21978@ZenIV.linux.org.uk

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [resend PATCH 0/2] fix VFS register_shrinker fixup

[Al Viro]

On Tue, Dec 19, 2017 at 04:19:15PM +0100, Michal Hocko wrote:
> Dohh, I have missed resend by Tetsuo http://lkml.kernel.org/r/1513596701-4518-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
> (thanks for dropping me from the CC).  Al seeemed to take the patch. We
> still need patch 2. Al, are you going to take it from this thread or you
> are going to go your way?

Umm... git log for-linus in vfs.git since yesterday:

commit 9ee332d99e4d5a97548943b81c54668450ce641b
Author: Al Viro <viro@zeniv.linux.org.uk>
Date:   Mon Dec 18 15:05:07 2017 -0500

    sget(): handle failures of register_shrinker()
    
    Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

commit bb422a738f6566f7439cd347d54e321e4fe92a9f
Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date:   Mon Dec 18 20:31:41 2017 +0900

    mm,vmscan: Make unregister_shrinker() no-op if register_shrinker() failed.
    
    Syzbot caught an oops at unregister_shrinker() because combination of
    commit 1d3d4437eae1bb29 ("vmscan: per-node deferred work") and fault
    injection made register_shrinker() fail and the caller of
    register_shrinker() did not check for failure.
....

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [resend PATCH 0/2] fix VFS register_shrinker fixup

[Michal Hocko]

On Tue 19-12-17 15:34:19, Al Viro wrote:
> On Tue, Dec 19, 2017 at 04:19:15PM +0100, Michal Hocko wrote:
> > Dohh, I have missed resend by Tetsuo http://lkml.kernel.org/r/1513596701-4518-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp
> > (thanks for dropping me from the CC).  Al seeemed to take the patch. We
> > still need patch 2. Al, are you going to take it from this thread or you
> > are going to go your way?
> 
> Umm... git log for-linus in vfs.git since yesterday:

OK, thanks!

> commit 9ee332d99e4d5a97548943b81c54668450ce641b
> Author: Al Viro <viro@zeniv.linux.org.uk>
> Date:   Mon Dec 18 15:05:07 2017 -0500
> 
>     sget(): handle failures of register_shrinker()
>     
>     Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> 
> commit bb422a738f6566f7439cd347d54e321e4fe92a9f
> Author: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date:   Mon Dec 18 20:31:41 2017 +0900
> 
>     mm,vmscan: Make unregister_shrinker() no-op if register_shrinker() failed.
>     
>     Syzbot caught an oops at unregister_shrinker() because combination of
>     commit 1d3d4437eae1bb29 ("vmscan: per-node deferred work") and fault
>     injection made register_shrinker() fail and the caller of
>     register_shrinker() did not check for failure.
> ....

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>