* Re: [Bug 177821] New: NULL pointer dereference in list_rcu [not found] <bug-177821-27@https.bugzilla.kernel.org/> @ 2016-10-17 23:56 ` Andrew Morton 2016-10-18 0:10 ` Andrew Morton 1 sibling, 0 replies; 4+ messages in thread From: Andrew Morton @ 2016-10-17 23:56 UTC (permalink / raw) To: Alexander Polakov; +Cc: bugzilla-daemon, Al Viro, Vladimir Davydov, linux-mm (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; } _ -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bug 177821] New: NULL pointer dereference in list_rcu [not found] <bug-177821-27@https.bugzilla.kernel.org/> 2016-10-17 23:56 ` [Bug 177821] New: NULL pointer dereference in list_rcu Andrew Morton @ 2016-10-18 0:10 ` Andrew Morton 2016-10-18 7:26 ` Alexander Polakov 1 sibling, 1 reply; 4+ messages in thread From: Andrew Morton @ 2016-10-18 0:10 UTC (permalink / raw) To: Alexander Polakov Cc: bugzilla-daemon, Al Viro, Vladimir Davydov, linux-mm, Vladimir Davydov (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; } _ -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bug 177821] New: NULL pointer dereference in list_rcu 2016-10-18 0:10 ` Andrew Morton @ 2016-10-18 7:26 ` Alexander Polakov 2016-10-18 16:41 ` Vladimir Davydov 0 siblings, 1 reply; 4+ messages in thread From: Alexander Polakov @ 2016-10-18 7:26 UTC (permalink / raw) To: Andrew Morton Cc: bugzilla-daemon, Al Viro, Vladimir Davydov, linux-mm, Vladimir Davydov > 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; > } > > _ > > -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Bug 177821] New: NULL pointer dereference in list_rcu 2016-10-18 7:26 ` Alexander Polakov @ 2016-10-18 16:41 ` Vladimir Davydov 0 siblings, 0 replies; 4+ messages in thread From: Vladimir Davydov @ 2016-10-18 16:41 UTC (permalink / raw) To: Alexander Polakov; +Cc: Andrew Morton, bugzilla-daemon, Al Viro, linux-mm On Tue, Oct 18, 2016 at 10:26:55AM +0300, Alexander Polakov wrote: > 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> Acked-by: Vladimir Davydov <vdavydov.dev@gmail.com> FWIW, The patch is indeed correct. However, failing a mount because of inability to allocate per memcg data sounds bad. We should probably fallback on vmalloc in memcg_{init,update}_list_lru_node() or use a contrived data structure, like flex_array, there. This is also fair for {init,update}_memcg_params in mm/slab_common.c. Thanks, Vladimir -- 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> ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-10-18 16:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <bug-177821-27@https.bugzilla.kernel.org/>
2016-10-17 23:56 ` [Bug 177821] New: NULL pointer dereference in list_rcu Andrew Morton
2016-10-18 0:10 ` Andrew Morton
2016-10-18 7:26 ` Alexander Polakov
2016-10-18 16:41 ` Vladimir Davydov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).