Bug 177821
| Summary: | NULL pointer dereference in list_rcu | ||
|---|---|---|---|
| Product: | Memory Management | Reporter: | Alexander Polakov (apolyakov) |
| Component: | Other | Assignee: | 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
(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; } _ (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; } _ > 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; > } > > _ > > |