[PATCH] mm/kfence: fix a potential NULL pointer dereference

cgel.zte
2022-04-27T07:11:18Z

From: xu xin <xu.xin16@zte.com.cn>

In __kfence_free(), the returned 'meta' from addr_to_metadata()
might be NULL just as the implementation of addr_to_metadata()
shows.

Let's add a check of the pointer 'meta' to avoid NULL pointer
dereference. The patch brings three changes:

1. Add checks in both kfence_free() and __kfence_free();
2. kfence_free is not inline function any longer and new inline
function '__try_free_kfence_meta' is introduced.
3. The check of is_kfence_address() is not required for
__kfence_free() now because __kfence_free has done the check in
addr_to_metadata();

Reported-by: Zeal Robot <zealci@zte.com.cn>
Signed-off-by: xu xin <xu.xin16@zte.com.cn>
---
include/linux/kfence.h | 10 ++--------
mm/kfence/core.c | 30 +++++++++++++++++++++++++++---
2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/include/linux/kfence.h b/include/linux/kfence.h
index 726857a4b680..fbf6391ab53c 100644
--- a/include/linux/kfence.h
+++ b/include/linux/kfence.h
@@ -160,7 +160,7 @@ void *kfence_object_start(const void *addr);
* __kfence_free() - release a KFENCE heap object to KFENCE pool
* @addr: object to be freed
*
- * Requires: is_kfence_address(addr)
+ * Requires: is_kfence_address(addr), but now it's unnecessary
*
* Release a KFENCE object and mark it as freed.
*/
@@ -179,13 +179,7 @@ void __kfence_free(void *addr);
* allocator's free codepath. The allocator must check the return value to
* determine if it was a KFENCE object or not.
*/
-static __always_inline __must_check bool kfence_free(void *addr)
-{
- if (!is_kfence_address(addr))
- return false;
- __kfence_free(addr);
- return true;
-}
+bool __must_check kfence_free(void *addr);

/**
* kfence_handle_page_fault() - perform page fault handling for KFENCE pages
diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 6e69986c3f0d..1405585369b3 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -1048,10 +1048,10 @@ void *kfence_object_start(const void *addr)
return meta ? (void *)meta->addr : NULL;
}

-void __kfence_free(void *addr)
-{
- struct kfence_metadata *meta = addr_to_metadata((unsigned long)addr);

+/* Require: meta is not NULL*/
+static __always_inline void __try_free_kfence_meta(struct kfence_metadata *meta)
+{
#ifdef CONFIG_MEMCG
KFENCE_WARN_ON(meta->objcg);
#endif
@@ -1067,6 +1067,30 @@ void __kfence_free(void *addr)
kfence_guarded_free(addr, meta, false);
}

+void __kfence_free(void *addr)
+{
+ struct kfence_metadata *meta = addr_to_metadata((unsigned long)addr);
+
+ if (!meta) {
+ kfence_report_error(addr, false, NULL, NULL, KFENCE_ERROR_INVALID);
+ return;
+ }
+
+ __try_free_kfence_meta(meta);
+}
+
+bool __must_check kfence_free(void *addr)
+{
+ struct kfence_metadata *meta = addr_to_metadata((unsigned long)addr);
+
+ if (!meta)
+ return false;
+
+ __try_free_kfence_meta(meta);
+
+ return true;
+}
+
bool kfence_handle_page_fault(unsigned long addr, bool is_write, struct pt_regs *regs)
{
const int page_index = (addr - (unsigned long)__kfence_pool) / PAGE_SIZE;
--
2.25.1