diff mbox series

MM: kmemleak: Removed coding style warnings and added a NULL check

Message ID 20231108062756.8129-1-ov.wagle@gmail.com (mailing list archive)
State New
Headers show
Series MM: kmemleak: Removed coding style warnings and added a NULL check | expand

Commit Message

Omkar Wagle Nov. 8, 2023, 6:27 a.m. UTC 
Fixed most of the coding style warnings
Added a NULL check to "object" pointer before accessing its members

Signed-off-by: Omkar Wagle <ov.wagle@gmail.com>
---
 mm/kmemleak.c | 50 +++++++++++++++++++++++++++-----------------------
 1 file changed, 27 insertions(+), 23 deletions(-)

Comments

Catalin Marinas Nov. 8, 2023, 10:41 a.m. UTC | #1 
On Tue, Nov 07, 2023 at 10:27:56PM -0800, Omkar Wagle wrote:
> @@ -406,10 +406,13 @@ static struct kmemleak_object *__lookup_object(unsigned long ptr, int alias,
>  	unsigned long untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
>  
>  	while (rb) {
> -		struct kmemleak_object *object;
> +		struct kmemleak_object *object = NULL;
>  		unsigned long untagged_objp;
>  
>  		object = rb_entry(rb, struct kmemleak_object, rb_node);
> +		if (!object)
> +			break;

No need for this. rb_entry() is a container_of() and we already check
that rb is not NULL.

> @@ -1027,7 +1030,7 @@ static void object_no_scan(unsigned long ptr)
>  void __ref kmemleak_alloc(const void *ptr, size_t size, int min_count,
>  			  gfp_t gfp)
>  {
> -	pr_debug("%s(0x%px, %zu, %d)\n", __func__, ptr, size, min_count);
> +	pr_debug("%s(0x%p, %zu, %d)\n", __func__, ptr, size, min_count);

That's for debugging, I'd rather keep the %px option. The same comment
applies to all the other pr_debug() changes in this file.
diff mbox series

Patch

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 1eacca03bedd..a7b74dc3ff01 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -1,6 +1,5 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
 /*
- * mm/kmemleak.c
  *
  * Copyright (C) 2008 ARM Limited
  * Written by Catalin Marinas <catalin.marinas@arm.com>
@@ -97,7 +96,7 @@ 
 #include <linux/crc32.h>
 
 #include <asm/sections.h>
-#include <asm/processor.h>
+#include <linux/processor.h>
 #include <linux/atomic.h>
 
 #include <linux/kasan.h>
@@ -368,6 +367,7 @@  static void print_unreferenced(struct seq_file *seq,
 
 	for (i = 0; i < nr_entries; i++) {
 		void *ptr = (void *)entries[i];
+
 		warn_or_seq_printf(seq, "    [<%pK>] %pS\n", ptr, ptr);
 	}
 }
@@ -406,10 +406,13 @@  static struct kmemleak_object *__lookup_object(unsigned long ptr, int alias,
 	unsigned long untagged_ptr = (unsigned long)kasan_reset_tag((void *)ptr);
 
 	while (rb) {
-		struct kmemleak_object *object;
+		struct kmemleak_object *object = NULL;
 		unsigned long untagged_objp;
 
 		object = rb_entry(rb, struct kmemleak_object, rb_node);
+		if (!object)
+			break;
+
 		untagged_objp = (unsigned long)kasan_reset_tag((void *)object->pointer);
 
 		if (untagged_ptr < untagged_objp)
@@ -674,10 +677,10 @@  static int __link_object(struct kmemleak_object *object, unsigned long ptr,
 	/* task information */
 	if (in_hardirq()) {
 		object->pid = 0;
-		strncpy(object->comm, "hardirq", sizeof(object->comm));
+		strscpy(object->comm, "hardirq", sizeof(object->comm));
 	} else if (in_serving_softirq()) {
 		object->pid = 0;
-		strncpy(object->comm, "softirq", sizeof(object->comm));
+		strscpy(object->comm, "softirq", sizeof(object->comm));
 	} else {
 		object->pid = current->pid;
 		/*
@@ -686,7 +689,7 @@  static int __link_object(struct kmemleak_object *object, unsigned long ptr,
 		 * dependency issues with current->alloc_lock. In the worst
 		 * case, the command line is not correct.
 		 */
-		strncpy(object->comm, current->comm, sizeof(object->comm));
+		strscpy(object->comm, current->comm, sizeof(object->comm));
 	}
 
 	/* kernel backtrace */
@@ -1027,7 +1030,7 @@  static void object_no_scan(unsigned long ptr)
 void __ref kmemleak_alloc(const void *ptr, size_t size, int min_count,
 			  gfp_t gfp)
 {
-	pr_debug("%s(0x%px, %zu, %d)\n", __func__, ptr, size, min_count);
+	pr_debug("%s(0x%p, %zu, %d)\n", __func__, ptr, size, min_count);
 
 	if (kmemleak_enabled && ptr && !IS_ERR(ptr))
 		create_object((unsigned long)ptr, size, min_count, gfp);
@@ -1048,7 +1051,7 @@  void __ref kmemleak_alloc_percpu(const void __percpu *ptr, size_t size,
 {
 	unsigned int cpu;
 
-	pr_debug("%s(0x%px, %zu)\n", __func__, ptr, size);
+	pr_debug("%s(0x%p, %zu)\n", __func__, ptr, size);
 
 	/*
 	 * Percpu allocations are only scanned and not reported as leaks
@@ -1072,7 +1075,7 @@  EXPORT_SYMBOL_GPL(kmemleak_alloc_percpu);
  */
 void __ref kmemleak_vmalloc(const struct vm_struct *area, size_t size, gfp_t gfp)
 {
-	pr_debug("%s(0x%px, %zu)\n", __func__, area, size);
+	pr_debug("%s(0x%p, %zu)\n", __func__, area, size);
 
 	/*
 	 * A min_count = 2 is needed because vm_struct contains a reference to
@@ -1095,7 +1098,7 @@  EXPORT_SYMBOL_GPL(kmemleak_vmalloc);
  */
 void __ref kmemleak_free(const void *ptr)
 {
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_free_enabled && ptr && !IS_ERR(ptr))
 		delete_object_full((unsigned long)ptr);
@@ -1113,7 +1116,7 @@  EXPORT_SYMBOL_GPL(kmemleak_free);
  */
 void __ref kmemleak_free_part(const void *ptr, size_t size)
 {
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_enabled && ptr && !IS_ERR(ptr))
 		delete_object_part((unsigned long)ptr, size, false);
@@ -1131,7 +1134,7 @@  void __ref kmemleak_free_percpu(const void __percpu *ptr)
 {
 	unsigned int cpu;
 
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_free_enabled && ptr && !IS_ERR(ptr))
 		for_each_possible_cpu(cpu)
@@ -1152,7 +1155,7 @@  void __ref kmemleak_update_trace(const void *ptr)
 	struct kmemleak_object *object;
 	unsigned long flags;
 
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (!kmemleak_enabled || IS_ERR_OR_NULL(ptr))
 		return;
@@ -1183,7 +1186,7 @@  EXPORT_SYMBOL(kmemleak_update_trace);
  */
 void __ref kmemleak_not_leak(const void *ptr)
 {
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_enabled && ptr && !IS_ERR(ptr))
 		make_gray_object((unsigned long)ptr);
@@ -1201,7 +1204,7 @@  EXPORT_SYMBOL(kmemleak_not_leak);
  */
 void __ref kmemleak_ignore(const void *ptr)
 {
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_enabled && ptr && !IS_ERR(ptr))
 		make_black_object((unsigned long)ptr, false);
@@ -1221,7 +1224,7 @@  EXPORT_SYMBOL(kmemleak_ignore);
  */
 void __ref kmemleak_scan_area(const void *ptr, size_t size, gfp_t gfp)
 {
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_enabled && ptr && size && !IS_ERR(ptr))
 		add_scan_area((unsigned long)ptr, size, gfp);
@@ -1239,7 +1242,7 @@  EXPORT_SYMBOL(kmemleak_scan_area);
  */
 void __ref kmemleak_no_scan(const void *ptr)
 {
-	pr_debug("%s(0x%px)\n", __func__, ptr);
+	pr_debug("%s(0x%p)\n", __func__, ptr);
 
 	if (kmemleak_enabled && ptr && !IS_ERR(ptr))
 		object_no_scan((unsigned long)ptr);
@@ -1255,7 +1258,7 @@  EXPORT_SYMBOL(kmemleak_no_scan);
  */
 void __ref kmemleak_alloc_phys(phys_addr_t phys, size_t size, gfp_t gfp)
 {
-	pr_debug("%s(0x%px, %zu)\n", __func__, &phys, size);
+	pr_debug("%s(0x%p, %zu)\n", __func__, &phys, size);
 
 	if (kmemleak_enabled)
 		/*
@@ -1275,7 +1278,7 @@  EXPORT_SYMBOL(kmemleak_alloc_phys);
  */
 void __ref kmemleak_free_part_phys(phys_addr_t phys, size_t size)
 {
-	pr_debug("%s(0x%px)\n", __func__, &phys);
+	pr_debug("%s(0x%p)\n", __func__, &phys);
 
 	if (kmemleak_enabled)
 		delete_object_part((unsigned long)phys, size, true);
@@ -1289,7 +1292,7 @@  EXPORT_SYMBOL(kmemleak_free_part_phys);
  */
 void __ref kmemleak_ignore_phys(phys_addr_t phys)
 {
-	pr_debug("%s(0x%px)\n", __func__, &phys);
+	pr_debug("%s(0x%p)\n", __func__, &phys);
 
 	if (kmemleak_enabled)
 		make_black_object((unsigned long)phys, true);
@@ -1662,6 +1665,7 @@  static void kmemleak_scan(void)
 		rcu_read_lock();
 		for_each_process_thread(g, p) {
 			void *stack = try_get_task_stack(p);
+
 			if (stack) {
 				scan_block(stack, stack + THREAD_SIZE, NULL);
 				put_task_stack(p);
@@ -1768,6 +1772,7 @@  static int kmemleak_scan_thread(void *arg)
 	 */
 	if (first_run) {
 		signed long timeout = msecs_to_jiffies(SECS_FIRST_SCAN * 1000);
+
 		first_run = 0;
 		while (timeout && !kthread_should_stop())
 			timeout = schedule_timeout_interruptible(timeout);
@@ -2013,7 +2018,7 @@  static ssize_t kmemleak_write(struct file *file, const char __user *user_buf,
 	else if (strncmp(buf, "scan=off", 8) == 0)
 		stop_scan_thread();
 	else if (strncmp(buf, "scan=", 5) == 0) {
-		unsigned secs;
+		unsigned int secs;
 		unsigned long msecs;
 
 		ret = kstrtouint(buf + 5, 0, &secs);
@@ -2130,8 +2135,7 @@  static int __init kmemleak_boot_config(char *str)
 	else if (strcmp(str, "on") == 0) {
 		kmemleak_skip_disable = 1;
 		stack_depot_request_early_init();
-	}
-	else
+	} else
 		return -EINVAL;
 	return 0;
 }