[PATCH 0/2] debugobjects: Fix NULL pointer dereference in kmem_alloc_batch()

[PATCH 0/2] debugobjects: Fix NULL pointer dereference in kmem_alloc_batch()

[Sang-Heon Jeon]

Hello,

During boot, kmem_alloc_batch() is called while obj_cache is still NULL.
If kmem_cache_zalloc() fails there, free_object_list() calls
kmem_cache_free() with obj_cache, which causes a NULL pointer dereference.

Patch 1 fixes it by passing the cache to free_object_list().

Patch 2 uses free_object_list() in the boot error path instead of open
coding the loop.

Sang-Heon Jeon (2):
  debugobjects: Fix NULL pointer dereference in kmem_alloc_batch()
  debugobjects: Use free_object_list() in the boot error path

 lib/debugobjects.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)


base-commit: 9f74092fda91ef93c2b07585785c1dc83cd7971f
-- 
2.53.0

[PATCH 1/2] debugobjects: Fix NULL pointer dereference in kmem_alloc_batch()

[Sang-Heon Jeon]

During boot, debug_objects_replace_static_objects() calls
kmem_alloc_batch() while obj_cache is still NULL.

If kmem_cache_zalloc() fails, kmem_alloc_batch() tries to free the
allocated objects with free_object_list(). But free_object_list()
always calls kmem_cache_free() with obj_cache, which causes a NULL
pointer dereference.

So use the cache passed to kmem_alloc_batch() instead.

Fixes: aebbfe0779b2 ("debugobjects: Prepare kmem_cache allocations for batching")
Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
 lib/debugobjects.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 877f7675ba26..7ab1fd8e0891 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -315,7 +315,7 @@ static void pcpu_free(struct debug_obj *obj)
 	WRITE_ONCE(pool_global.stats.cur_used, pool_global.stats.cur_used - ODEBUG_BATCH_SIZE);
 }
 
-static void free_object_list(struct hlist_head *head)
+static void free_object_list(struct hlist_head *head, struct kmem_cache *cache)
 {
 	struct hlist_node *tmp;
 	struct debug_obj *obj;
@@ -323,7 +323,7 @@ static void free_object_list(struct hlist_head *head)
 
 	hlist_for_each_entry_safe(obj, tmp, head, node) {
 		hlist_del(&obj->node);
-		kmem_cache_free(obj_cache, obj);
+		kmem_cache_free(cache, obj);
 		cnt++;
 	}
 	debug_objects_freed += cnt;
@@ -370,7 +370,7 @@ static bool kmem_alloc_batch(struct hlist_head *head, struct kmem_cache *cache,
 	for (int cnt = 0; cnt < ODEBUG_BATCH_SIZE; cnt++) {
 		obj = kmem_cache_zalloc(cache, gfp);
 		if (!obj) {
-			free_object_list(head);
+			free_object_list(head, cache);
 			return false;
 		}
 		debug_objects_allocated++;
@@ -527,7 +527,7 @@ static void free_obj_work(struct work_struct *work)
 				return;
 			}
 		}
-		free_object_list(&tofree);
+		free_object_list(&tofree, obj_cache);
 	}
 }
 
-- 
2.53.0

[PATCH 2/2] debugobjects: Use free_object_list() in the boot error path

[Sang-Heon Jeon]

Now that free_object_list() takes a cache argument, use it in the error
path of debug_objects_replace_static_objects() instead of open coding
the loop.

Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
---
 lib/debugobjects.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/lib/debugobjects.c b/lib/debugobjects.c
index 7ab1fd8e0891..b066036ff37e 100644
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -1502,7 +1502,6 @@ void __init debug_objects_early_init(void)
 static bool __init debug_objects_replace_static_objects(struct kmem_cache *cache)
 {
 	struct debug_bucket *db = obj_hash;
-	struct hlist_node *tmp;
 	struct debug_obj *obj;
 	HLIST_HEAD(objects);
 	int i;
@@ -1530,11 +1529,7 @@ static bool __init debug_objects_replace_static_objects(struct kmem_cache *cache
 	}
 	return true;
 free:
-	/* Can't use free_object_list() as the cache is not populated yet */
-	hlist_for_each_entry_safe(obj, tmp, &pool_global.objects, node) {
-		hlist_del(&obj->node);
-		kmem_cache_free(cache, obj);
-	}
+	free_object_list(&pool_global.objects, cache);
 	return false;
 }
 
-- 
2.53.0