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

[Vasily Averin]

On 22.12.2021 22:48, Manfred Spraul 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.
> 
> Reported-by: Zeal Robot <zealci@xxxxxxxxxx>
> Reported-by: Minghao Chi <chi.minghao@xxxxxxxxxx>
> Link: https://lore.kernel.org/all/20211222081026.484058-1-chi.minghao@xxxxxxxxxx/
> Fixes: fc37a3b8b438 ("[PATCH] ipc sem: use kvmalloc for sem_undo allocation")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Manfred Spraul <manfred@xxxxxxxxxxxxxxxx>
> ---
>  mm/util.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/util.c b/mm/util.c
> index 741ba32a43ac..7f9181998835 100644
> --- 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);
>  }

I would prefer to release memory ASAP if it's possible.
What do you think about this change?
--- a/mm/util.c
+++ b/mm/util.c
@@ -614,9 +614,12 @@ EXPORT_SYMBOL(kvmalloc_node);
  */
 void kvfree(const void *addr)
 {
-       if (is_vmalloc_addr(addr))
-               vfree(addr);
-       else
+       if (is_vmalloc_addr(addr)) {
+               if (in_atomic())
+                       vfree_atomic();
+               else
+                       vfree(addr);
+       } else
                kfree(addr);
 }
 EXPORT_SYMBOL(kvfree);