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@suse.de
Reported-by: Zeal Robot zealci@zte.com.cn Reported-by: Minghao Chi chi.minghao@zte.com.cn Link: https://lore.kernel.org/all/20211222081026.484058-1-chi.minghao@zte.com.cn/ Fixes: fc37a3b8b438 ("[PATCH] ipc sem: use kvmalloc for sem_undo allocation") Cc: stable@vger.kernel.org Signed-off-by: Manfred Spraul manfred@colorfullife.com
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);}