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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1608517898.9171.6.camel@mtksdccf07>
Date:   Mon, 21 Dec 2020 10:31:38 +0800
From:   Kuan-Ying Lee <Kuan-Ying.Lee@...iatek.com>
To:     Stephen Rothwell <sfr@...b.auug.org.au>
CC:     Andrew Morton <akpm@...ux-foundation.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux Next Mailing List <linux-next@...r.kernel.org>
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 <sfr@...b.auug.org.au>
> Date: Mon, 21 Dec 2020 13:07:42 +1100
> Subject: [PATCH] kasan: fix memory leak of kasan quarantine fix
> 
> Signed-off-by: Stephen Rothwell <sfr@...b.auug.org.au>
> ---
>  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.