2020-12-21 02:13:36

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: build failure after merge of the akpm-current tree

Hi all,

After merging the akpm-current tree, today's linux-next build (x86_64
allmodconfig) failed like this:

mm/kasan/quarantine.c: In function 'quarantine_put':
mm/kasan/quarantine.c:207:15: error: 'info' undeclared (first use in this function)
207 | qlink_free(&info->quarantine_link, cache);
| ^~~~

Caused by commit

120d593a8650 ("kasan: fix memory leak of kasan quarantine")

interacting with commit

cfbc92088e1d ("kasan: rename get_alloc/free_info")

Can we please get this sorted out once and for all?

I have applied the following patch for today:

From: Stephen Rothwell <[email protected]>
Date: Mon, 21 Dec 2020 13:07:42 +1100
Subject: [PATCH] kasan: fix memory leak of kasan quarantine fix

Signed-off-by: Stephen Rothwell <[email protected]>
---
mm/kasan/quarantine.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
index 3f3b3d902c18..091a57f942b3 100644
--- a/mm/kasan/quarantine.c
+++ b/mm/kasan/quarantine.c
@@ -204,7 +204,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)

q = this_cpu_ptr(&cpu_quarantine);
if (q->offline) {
- qlink_free(&info->quarantine_link, cache);
+ qlink_free(&meta->quarantine_link, cache);
local_irq_restore(flags);
return false;
}
--
2.29.2

--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature

Subject: Re: linux-next: build failure after merge of the akpm-current tree

On Mon, 2020-12-21 at 13:10 +1100, Stephen Rothwell wrote:
> Hi all,
>
> After merging the akpm-current tree, today's linux-next build (x86_64
> allmodconfig) failed like this:
>
> mm/kasan/quarantine.c: In function 'quarantine_put':
> mm/kasan/quarantine.c:207:15: error: 'info' undeclared (first use in this function)
> 207 | qlink_free(&info->quarantine_link, cache);
> | ^~~~
>
> Caused by commit
>
> 120d593a8650 ("kasan: fix memory leak of kasan quarantine")
>
> interacting with commit
>
> cfbc92088e1d ("kasan: rename get_alloc/free_info")
>
> Can we please get this sorted out once and for all?
>
> I have applied the following patch for today:
>
> From: Stephen Rothwell <[email protected]>
> Date: Mon, 21 Dec 2020 13:07:42 +1100
> Subject: [PATCH] kasan: fix memory leak of kasan quarantine fix
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> mm/kasan/quarantine.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
> index 3f3b3d902c18..091a57f942b3 100644
> --- a/mm/kasan/quarantine.c
> +++ b/mm/kasan/quarantine.c
> @@ -204,7 +204,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)
>
> q = this_cpu_ptr(&cpu_quarantine);
> if (q->offline) {
> - qlink_free(&info->quarantine_link, cache);
> + qlink_free(&meta->quarantine_link, cache); // free once
> local_irq_restore(flags);
> return false; // free twice
> }
> --
> 2.29.2
>

Hi Stephen,
Please just drop this patch "kasan: fix memory leak of kasan quarantine"
from linux-next. Otherwise, it would cause double free issue.


Hi Andrew,

Sorry to bother.
I upload the v2 standalone fixup patch to fix the memory leak issue on
kernel-5.10 stable as below.
https://marc.info/?l=linux-mm&m=160820751825252&w=2
I think this slab memory leak issue is important. It's because when we
do kmem_cache_destroy, it will report object remaining error.

Add this v2 patch to mm-tree, it will have conflicts with
Andrey's patches as below.
"kasan: rename get_alloc/free_info"
"kasan: sanitize objects when metadata doesnt fit"

I think this standalone fixup patch should be added ""before"" Andrey's
patch in mm-tree. Because only merging this standalone fix patch to 5.10
stable, we can resolve this leak issue instead of merging the whole
patchset of Andrey's patch to 5.10 stable.
However, merging the fixup patch into mm-tree will cause some conflicts
in mm-tree.

Please help to fix the conflicts.
And I think the conflict between standalone fixup patch and
Andrey's patches will be fixed as below.

I think this patch "kasan: rename get_alloc/free_info" need to rename
the "info" to "meta" as below.

- qlink_free(&info->quarantine_link, cache);
+ qlink_free(&meta->quarantine_link, cache);


This patch "kasan: sanitize objects when metadata doesnt fit" need to
remove the qlink_free() and add return false as below.

q = this_cpu_ptr(&cpu_quarantine);
if (q->offline) {
- qlink_free(&meta->quarantine_link, cache);
local_irq_restore(flags);
- return;
+ return false;
}

Thanks a lot.

2020-12-21 02:59:06

by Stephen Rothwell

[permalink] [raw]
Subject: Re: linux-next: build failure after merge of the akpm-current tree

Hi Kuan-Ying,

On Mon, 21 Dec 2020 10:31:38 +0800 Kuan-Ying Lee <[email protected]> wrote:
>
> On Mon, 2020-12-21 at 13:10 +1100, Stephen Rothwell wrote:
> >
> > After merging the akpm-current tree, today's linux-next build (x86_64
> > allmodconfig) failed like this:
> >
> > mm/kasan/quarantine.c: In function 'quarantine_put':
> > mm/kasan/quarantine.c:207:15: error: 'info' undeclared (first use in this function)
> > 207 | qlink_free(&info->quarantine_link, cache);
> > | ^~~~
> >
> > Caused by commit
> >
> > 120d593a8650 ("kasan: fix memory leak of kasan quarantine")
> >
> > interacting with commit
> >
> > cfbc92088e1d ("kasan: rename get_alloc/free_info")
> >
> > Can we please get this sorted out once and for all?
> >
> > I have applied the following patch for today:
> >
> > From: Stephen Rothwell <[email protected]>
> > Date: Mon, 21 Dec 2020 13:07:42 +1100
> > Subject: [PATCH] kasan: fix memory leak of kasan quarantine fix
> >
> > Signed-off-by: Stephen Rothwell <[email protected]>
> > ---
> > mm/kasan/quarantine.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
> > index 3f3b3d902c18..091a57f942b3 100644
> > --- a/mm/kasan/quarantine.c
> > +++ b/mm/kasan/quarantine.c
> > @@ -204,7 +204,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)
> >
> > q = this_cpu_ptr(&cpu_quarantine);
> > if (q->offline) {
> > - qlink_free(&info->quarantine_link, cache);
> > + qlink_free(&meta->quarantine_link, cache); // free once
> > local_irq_restore(flags);
> > return false; // free twice
> > }
> > --
> > 2.29.2
> >
>
> Please just drop this patch "kasan: fix memory leak of kasan quarantine"
> from linux-next. Otherwise, it would cause double free issue.

OK, so for today I have reverted my fix patch and 120d593a8650 ("kasan:
fix memory leak of kasan quarantine").

--
Cheers,
Stephen Rothwell


Attachments:
(No filename) (499.00 B)
OpenPGP digital signature
Subject: Re: linux-next: build failure after merge of the akpm-current tree

On Mon, 2020-12-21 at 13:55 +1100, Stephen Rothwell wrote:
> Hi Kuan-Ying,
>
> On Mon, 21 Dec 2020 10:31:38 +0800 Kuan-Ying Lee <[email protected]> wrote:
> >
> > On Mon, 2020-12-21 at 13:10 +1100, Stephen Rothwell wrote:
> > >
> > > After merging the akpm-current tree, today's linux-next build (x86_64
> > > allmodconfig) failed like this:
> > >
> > > mm/kasan/quarantine.c: In function 'quarantine_put':
> > > mm/kasan/quarantine.c:207:15: error: 'info' undeclared (first use in this function)
> > > 207 | qlink_free(&info->quarantine_link, cache);
> > > | ^~~~
> > >
> > > Caused by commit
> > >
> > > 120d593a8650 ("kasan: fix memory leak of kasan quarantine")
> > >
> > > interacting with commit
> > >
> > > cfbc92088e1d ("kasan: rename get_alloc/free_info")
> > >
> > > Can we please get this sorted out once and for all?
> > >
> > > I have applied the following patch for today:
> > >
> > > From: Stephen Rothwell <[email protected]>
> > > Date: Mon, 21 Dec 2020 13:07:42 +1100
> > > Subject: [PATCH] kasan: fix memory leak of kasan quarantine fix
> > >
> > > Signed-off-by: Stephen Rothwell <[email protected]>
> > > ---
> > > mm/kasan/quarantine.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/mm/kasan/quarantine.c b/mm/kasan/quarantine.c
> > > index 3f3b3d902c18..091a57f942b3 100644
> > > --- a/mm/kasan/quarantine.c
> > > +++ b/mm/kasan/quarantine.c
> > > @@ -204,7 +204,7 @@ bool quarantine_put(struct kmem_cache *cache, void *object)
> > >
> > > q = this_cpu_ptr(&cpu_quarantine);
> > > if (q->offline) {
> > > - qlink_free(&info->quarantine_link, cache);
> > > + qlink_free(&meta->quarantine_link, cache); // free once
> > > local_irq_restore(flags);
> > > return false; // free twice
> > > }
> > > --
> > > 2.29.2
> > >
> >
> > Please just drop this patch "kasan: fix memory leak of kasan quarantine"
> > from linux-next. Otherwise, it would cause double free issue.
>
> OK, so for today I have reverted my fix patch and 120d593a8650 ("kasan:
> fix memory leak of kasan quarantine").
>

Dear Andrew,

I am sorry. I didn't mean to.

This patch has build error.
https://www.ozlabs.org/~akpm/mmotm/broken-out/kasan-fix-memory-leak-of-kasan-quarantine.patch


Sorry to make the build errors and merge issues repeatedly.
My fix has dependency issues with Andrey's patches [1, 2], and I think
it's better to merge Andrey's patches first and I will push a fix after
Andrey's patch.
Please just drop this patch from akpm-tree directly.

[1]https://www.ozlabs.org/~akpm/mmotm/broken-out/kasan-sanitize-objects-when-metadata-doesnt-fit.patch
[2]https://www.ozlabs.org/~akpm/mmotm/broken-out/kasan-rename-get_alloc-free_info.patch


Thanks.