linux-kernel - Re: [PATCH] mm/util.c: Make kvfree() safe for calling while holding spinlocks

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: <20220126185340.58f88e8e1b153b6650c83270@linux-foundation.org>
Date:   Wed, 26 Jan 2022 18:53:40 -0800
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Manfred Spraul <manfred@...orfullife.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Vasily Averin <vvs@...tuozzo.com>, cgel.zte@...il.com,
        shakeelb@...gle.com, rdunlap@...radead.org, dbueso@...e.de,
        unixbhaskar@...il.com, chi.minghao@....com.cn, arnd@...db.de,
        Zeal Robot <zealci@....com.cn>, linux-mm@...ck.org,
        1vier1@....de, stable@...r.kernel.org,
        Michal Hocko <mhocko@...nel.org>
Subject: Re: [PATCH] mm/util.c: Make kvfree() safe for calling while holding
 spinlocks

On Wed, 22 Dec 2021 20:48:28 +0100 Manfred Spraul <manfred@...orfullife.com> wrote:

> One codepath in find_alloc_undo() calls kvfree() while holding a spinlock.
> Since vfree() can sleep this is a bug.
> 
> Previously, the code path used kfree(), and kfree() is safe to be called
> while holding a spinlock.
> 
> Minghao proposed to fix this by updating find_alloc_undo().
> 
> Alternate proposal to fix this: Instead of changing find_alloc_undo(),
> change kvfree() so that the same rules as for kfree() apply:
> Having different rules for kfree() and kvfree() just asks for bugs.
> 
> Disadvantage: Releasing vmalloc'ed memory will be delayed a bit.

I know we've been around this loop a bunch of times and deferring was
considered.   But I forget the conclusion.  IIRC, mhocko was involved?

> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -610,12 +610,12 @@ EXPORT_SYMBOL(kvmalloc_node);
>   * It is slightly more efficient to use kfree() or vfree() if you are certain
>   * that you know which one to use.
>   *
> - * Context: Either preemptible task context or not-NMI interrupt.
> + * Context: Any context except NMI interrupt.
>   */
>  void kvfree(const void *addr)
>  {
>  	if (is_vmalloc_addr(addr))
> -		vfree(addr);
> +		vfree_atomic(addr);
>  	else
>  		kfree(addr);
>  }