The kmemleak object is allocated by mem_pool_alloc(), which
could be from slab or mem_pool[], so it's not suitable using
__kmem_cache_free() to free the object, use __mem_pool_free()
instead.

Fixes: 0647398a8c7b ("mm: kmemleak: simple memory allocation pool for kmemleak objects")
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
 mm/kmemleak.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 064fc3695c6b..ea34986c02b4 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -668,8 +668,8 @@ static struct kmemleak_object * __alloc_object(gfp_t gfp)
 	return object;
 }
 
-static void __link_object(struct kmemleak_object *object, unsigned long ptr,
-			  size_t size, int min_count, bool is_phys)
+static int __link_object(struct kmemleak_object *object, unsigned long ptr,
+			 size_t size, int min_count, bool is_phys)
 {
 
 	struct kmemleak_object *parent;
@@ -711,14 +711,15 @@ static void __link_object(struct kmemleak_object *object, unsigned long ptr,
 			 * be freed while the kmemleak_lock is held.
 			 */
 			dump_object_info(parent);
-			kmem_cache_free(object_cache, object);
-			return;
+			return -EEXIST;
 		}
 	}
 	rb_link_node(&object->rb_node, rb_parent, link);
 	rb_insert_color(&object->rb_node, is_phys ? &object_phys_tree_root :
 					  &object_tree_root);
 	list_add_tail_rcu(&object->object_list, &object_list);
+
+	return 0;
 }
 
 /*
@@ -731,14 +732,17 @@ static void __create_object(unsigned long ptr, size_t size,
 {
 	struct kmemleak_object *object;
 	unsigned long flags;
+	int ret;
 
 	object = __alloc_object(gfp);
 	if (!object)
 		return;
 
 	raw_spin_lock_irqsave(&kmemleak_lock, flags);
-	__link_object(object, ptr, size, min_count, is_phys);
+	ret = __link_object(object, ptr, size, min_count, is_phys);
 	raw_spin_unlock_irqrestore(&kmemleak_lock, flags);
+	if (ret)
+		mem_pool_free(object);
 }
 
 /* Create kmemleak object which allocated with virtual address. */
-- 
2.25.1

On Wed, Oct 18, 2023 at 06:29:50PM +0800, Liu Shixin wrote:
> The kmemleak object is allocated by mem_pool_alloc(), which
> could be from slab or mem_pool[], so it's not suitable using
> __kmem_cache_free() to free the object, use __mem_pool_free()
> instead.
> 
> Fixes: 0647398a8c7b ("mm: kmemleak: simple memory allocation pool for kmemleak objects")
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>

Could you please reorder this patch before the previous one? If you
added a Fixes tag, we may want a cc stable as well (as for the other
patches with a Fixes tag) and it makes more sense to backport it on its
own without the __create_object() split. Otherwise:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

On Wed, Oct 18, 2023 at 04:48:06PM +0100, Catalin Marinas wrote:
> On Wed, Oct 18, 2023 at 06:29:50PM +0800, Liu Shixin wrote:
> > The kmemleak object is allocated by mem_pool_alloc(), which
> > could be from slab or mem_pool[], so it's not suitable using
> > __kmem_cache_free() to free the object, use __mem_pool_free()
> > instead.
> > 
> > Fixes: 0647398a8c7b ("mm: kmemleak: simple memory allocation pool for kmemleak objects")
> > Signed-off-by: Liu Shixin <liushixin2@huawei.com>
> 
> Could you please reorder this patch before the previous one? If you
> added a Fixes tag, we may want a cc stable as well (as for the other
> patches with a Fixes tag) and it makes more sense to backport it on its
> own without the __create_object() split. Otherwise:

Ah, ignore this. If we want a cc stable, the whole thing needs
backporting, including the split which is essential for the subsequent
fix.

> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

-- 
Catalin

On Wed, 18 Oct 2023 16:57:50 +0100 Catalin Marinas <catalin.marinas@arm.com> wrote:

> > Could you please reorder this patch before the previous one? If you
> > added a Fixes tag, we may want a cc stable as well (as for the other
> > patches with a Fixes tag) and it makes more sense to backport it on its
> > own without the __create_object() split. Otherwise:
> 
> Ah, ignore this. If we want a cc stable, the whole thing needs
> backporting, including the split which is essential for the subsequent
> fix.

Do we want a cc:stable?  That tag wasn't originally included.

If so, all seven patches?

If "not all seven" then can we please have two series, one for the
backport patches, one for next merge window.

On Wed, Oct 18, 2023 at 09:22:53AM -0700, Andrew Morton wrote:
> On Wed, 18 Oct 2023 16:57:50 +0100 Catalin Marinas <catalin.marinas@arm.com> wrote:
> > > Could you please reorder this patch before the previous one? If you
> > > added a Fixes tag, we may want a cc stable as well (as for the other
> > > patches with a Fixes tag) and it makes more sense to backport it on its
> > > own without the __create_object() split. Otherwise:
> > 
> > Ah, ignore this. If we want a cc stable, the whole thing needs
> > backporting, including the split which is essential for the subsequent
> > fix.
> 
> Do we want a cc:stable?  That tag wasn't originally included.
> 
> If so, all seven patches?
> 
> If "not all seven" then can we please have two series, one for the
> backport patches, one for next merge window.

I think we need all 7 if we are to backport them. But we don't need to
cc stable explicitly, we can send them to stable@kernel.org separately
once tested on those stable versions. So, for the mm tree, don't bother
with cc stable.

-- 
Catalin