| Message ID | 20231018102952.3339837-6-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Some bugfix about kmemleak | expand |
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>
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.
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. */
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(-)