| Message ID | 20200910100406.758051-2-kleber.souza@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Partial fix for mm/slub regression (LP #1895109) | expand |
Yeah, the memory leak is certainly better than the bug, though I can't see how
that commit would cause the bug in question.
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
On Thu, Sep 10, 2020 at 12:04:06PM +0200, Kleber Sacilotto de Souza wrote: > BugLink: https://bugs.launchpad.net/bugs/1895109 > > This reverts commit 8d392dd31bd9205986b442528014c6eec3851491 (commit > dde3c6b72a16c2db826f54b2d49bdea26c3534a2 upstream). > > It has been found that linux 5.4.0-45 introduced a regression that is > causing oops when starting LVM snapshots (LP #1894780). A fix for this > issue hasn't been found yet, but reverting dde3c6b72a16 "mm/slub: fix a > memory leak in sysfs_slab_add()" at least doesn't crash one of the > affected systems anymore. > > Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > --- > mm/slub.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/mm/slub.c b/mm/slub.c > index 709e31002504..434e4c6aca80 100644 > --- a/mm/slub.c > +++ b/mm/slub.c > @@ -5804,10 +5804,8 @@ static int sysfs_slab_add(struct kmem_cache *s) > > s->kobj.kset = kset; > err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, "%s", name); > - if (err) { > - kobject_put(&s->kobj); > + if (err) > goto out; > - } > > err = sysfs_create_group(&s->kobj, &slab_attr_group); > if (err) > -- > 2.25.1 Ugg. Acked-by: Andy Whitcroft <apw@canonical.com> -apw
On Thu, Sep 10, 2020 at 07:10:06AM -0300, Thadeu Lima de Souza Cascardo wrote: > Yeah, the memory leak is certainly better than the bug, though I can't see how > that commit would cause the bug in question. > > Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team Okay, so I looked at the bug and it all makes sense now. I am glad we proceeded with the revert: So, kobject_put will call the ktype release, which is mm/slub.c:kmem_cache_release, which calls mm/slab_common.c:slab_kmem_cache_release, which calls mm/slub.c:__kmem_cache_release. After we sysfs_slab_add returns failure, we call __kmem_cache_release again, which causes the double free. So, there is still the case of properly fixing this memleak the reverted commit claims to fix, and also avoiding creating the duplicate object. Cascardo.
diff --git a/mm/slub.c b/mm/slub.c index 709e31002504..434e4c6aca80 100644 --- a/mm/slub.c +++ b/mm/slub.c @@ -5804,10 +5804,8 @@ static int sysfs_slab_add(struct kmem_cache *s) s->kobj.kset = kset; err = kobject_init_and_add(&s->kobj, &slab_ktype, NULL, "%s", name); - if (err) { - kobject_put(&s->kobj); + if (err) goto out; - } err = sysfs_create_group(&s->kobj, &slab_attr_group); if (err)
BugLink: https://bugs.launchpad.net/bugs/1895109 This reverts commit 8d392dd31bd9205986b442528014c6eec3851491 (commit dde3c6b72a16c2db826f54b2d49bdea26c3534a2 upstream). It has been found that linux 5.4.0-45 introduced a regression that is causing oops when starting LVM snapshots (LP #1894780). A fix for this issue hasn't been found yet, but reverting dde3c6b72a16 "mm/slub: fix a memory leak in sysfs_slab_add()" at least doesn't crash one of the affected systems anymore. Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> --- mm/slub.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)