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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Cc'ing more mm folks.

On 2021-12-22 11:48, Manfred Spraul wrote:
One codepath in find_alloc_undo() calls kvfree() while holding a spinlock.
Since vfree() can sleep this is a bug.

afaict the only other offender is devx_async_cmd_event_destroy_uobj(), in drivers/infiniband/hw/mlx5/devx.c. I was expecting to find more, actually.

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.

I agree that it is best to have the same atomic semantics across all family of calls.


Disadvantage: Releasing vmalloc'ed memory will be delayed a bit.

I would not expect the added latency to be a big deal unless under serious memory pressure, for which case things are already fragile to begin with. Furthermore users of kvfree() are already warned that this is the slower choice. Feel free to add my:

Acked-by: Davidlohr Bueso <dbueso@xxxxxxx>


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);
 }



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux