Linux kbuild/kconfig development
 help / color / mirror / Atom feed
From: Li Chen <me@linux.beauty>
To: Kees Cook <kees@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas.schier@linux.dev>,
	linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org,
	linux-kbuild@vger.kernel.org
Subject: [RFC PATCH 2/2] gcc-plugins: cleanup_plugin: detect NULL init
Date: Wed,  5 Nov 2025 16:46:57 +0800	[thread overview]
Message-ID: <20251105084733.3598704-7-me@linux.beauty> (raw)
In-Reply-To: <20251105084733.3598704-1-me@linux.beauty>

From: Li Chen <chenl311@chinatelecom.cn>

The cleanup_plugin should warn about both uninitialized cleanup variables
and those initialized to NULL, as documented in include/linux/cleanup.h.
The "__free(...) = NULL" pattern at function top poses interdependency
problems.

Also, plugin warnings should not be converted to errors by -Werror, as
they are informational warnings meant to guide developers, not block
builds.

Signed-off-by: Li Chen <chenl311@chinatelecom.cn>
---
 scripts/gcc-plugins/Kconfig          |   2 +-
 scripts/gcc-plugins/cleanup_plugin.c | 122 ++++++++++++++++++++++++---
 2 files changed, 111 insertions(+), 13 deletions(-)

diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 906d50eb5efa6..ae6f79c617149 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -40,6 +40,6 @@ config GCC_PLUGIN_CLEANUP_ATTRIBUTE_WARN
 	def_bool y
 	help
 	  Warn when local automatic variables annotated with
-	  __attribute__((cleanup(...))) are declared without an initializer.
+	  __attribute__((cleanup(...))) are declared without an constructor.
 
 endif
diff --git a/scripts/gcc-plugins/cleanup_plugin.c b/scripts/gcc-plugins/cleanup_plugin.c
index d28f8969186de..c0744bbb7ef15 100644
--- a/scripts/gcc-plugins/cleanup_plugin.c
+++ b/scripts/gcc-plugins/cleanup_plugin.c
@@ -1,7 +1,69 @@
 // SPDX-License-Identifier: GPL-2.0
 /*
- * Warn about uninitialized automatic variables that use the
- * __attribute__((cleanup(...))) attribute.
+ * Copyright 2025 by Li Chen <me@linux.beauty>
+ *
+ * This gcc plugin warns about problematic patterns when using variables
+ * with __attribute__((cleanup(...))). The cleanup attribute helpers
+ * (__free, DEFINE_FREE, etc.) are designed to automatically clean up
+ * resources when variables go out of scope, following LIFO ordering.
+ * However, certain patterns can lead to interdependency issues.
+ *
+ * The plugin detects two problematic patterns:
+ *
+ * 1. Uninitialized cleanup variables:
+ *    Variables declared with cleanup attributes but not initialized can
+ *    cause issues when cleanup functions are called on undefined values.
+ *
+ *    Example:
+ *    void func(void)
+ *    {
+ *        struct resource *res __free(cleanup);  // Warning: not initialized
+ *        res = acquire_resource();
+ *        // ...
+ *    }
+ *
+ *    Should be:
+ *    void func(void)
+ *    {
+ *        struct resource *res __free(cleanup) = acquire_resource();
+ *        // ...
+ *    }
+ *
+ * 2. NULL-initialized cleanup variables:
+ *    The "__free(...) = NULL" pattern at function top can cause
+ *    interdependency problems, especially when combined with guards or
+ *    multiple cleanup variables, as documented in include/linux/cleanup.h.
+ *
+ *    Example:
+ *    void func(void)
+ *    {
+ *        struct resource *res __free(cleanup) = NULL;  // Warning: NULL init
+ *        guard(mutex)(&lock);
+ *        res = acquire_resource();
+ *        // cleanup may run without lock held!
+ *    }
+ *
+ *    Should be:
+ *    void func(void)
+ *    {
+ *        guard(mutex)(&lock);
+ *        struct resource *res __free(cleanup) = acquire_resource();
+ *        // ...
+ *    }
+ *
+ * The plugin provides clear warnings to help developers identify these
+ * patterns during compilation. Importantly, these warnings are not
+ * converted to errors by -Werror, allowing builds to continue while
+ * still alerting developers to potential issues.
+ *
+ * Options:
+ * - None currently supported
+ *
+ * Attribute: __attribute__((cleanup(...)))
+ *  The cleanup gcc attribute can be used on automatic variables to
+ *  specify a function to be called when the variable goes out of scope.
+ *  This plugin validates that such variables are properly initialized
+ *  at declaration time to avoid interdependency issues.
  */
 
 #include "gcc-common.h"
@@ -41,38 +103,74 @@ static bool is_candidate_decl(tree var)
 	return true;
 }
 
-static bool has_declaration_initializer(tree var)
+static bool is_null_initializer(tree initial)
 {
-	if (DECL_INITIAL(var))
+	if (!initial)
+		return false;
+
+	/* Check if the initializer is NULL pointer constant */
+	if (initial == null_pointer_node)
 		return true;
 
-#ifdef DECL_INITIALIZED_P
-	if (DECL_INITIALIZED_P(var))
+	/* Check if it's an integer constant zero (which can be NULL) */
+	if (TREE_CODE(initial) == INTEGER_CST && integer_zerop(initial))
 		return true;
-#endif
 
 	return false;
 }
 
+static bool has_valid_declaration_initializer(tree var)
+{
+	tree initial = DECL_INITIAL(var);
+
+	/* No initializer at all */
+	if (!initial) {
+#ifdef DECL_INITIALIZED_P
+		if (DECL_INITIALIZED_P(var))
+			return true;
+#endif
+		return false;
+	}
+
+	/* NULL initialization is considered invalid for cleanup variables */
+	if (is_null_initializer(initial))
+		return false;
+
+	/* Any other non-NULL initializer is valid */
+	return true;
+}
+
 static void warn_if_uninitialized(tree var)
 {
 	location_t loc;
 	bool saved_warning_as_error;
+	tree initial = DECL_INITIAL(var);
+	bool is_null_init = false;
 
-	if (has_declaration_initializer(var))
+	if (has_valid_declaration_initializer(var))
 		return;
 
 	loc = DECL_SOURCE_LOCATION(var);
 	if (loc == UNKNOWN_LOCATION)
 		return;
 
+	/* Check if it's a NULL initialization */
+	is_null_init = initial && is_null_initializer(initial);
+
 	/* Temporarily disable treating warnings as errors for this specific warning */
 	saved_warning_as_error = global_dc->warning_as_error_requested_p();
 	global_dc->set_warning_as_error_requested(false);
-	warning_at(
-		loc, 0,
-		"%qD declared with cleanup attribute is not initialized at declaration",
-		var);
+	if (is_null_init) {
+		warning_at(
+			loc, 0,
+			"%qD declared with cleanup attribute is initialized to NULL at declaration",
+			var);
+	} else {
+		warning_at(
+			loc, 0,
+			"%qD declared with cleanup attribute is not initialized at declaration",
+			var);
+	}
 	/* Restore the original setting */
 	global_dc->set_warning_as_error_requested(saved_warning_as_error);
 }
-- 
2.51.0


  parent reply	other threads:[~2025-11-05  8:49 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-05  8:46 [PATCH 0/3] dm-pcache: built-in support and metadata hardening Li Chen
2025-11-05  8:46 ` [PATCH 1/3] dm-pcache: allow built-in build and rename flush helper Li Chen
2025-11-05  8:46 ` [PATCH 2/3] dm-pcache: reuse meta_addr in pcache_meta_find_latest Li Chen
2025-11-05  8:46 ` [PATCH 3/3] dm-pcache: avoid leaking invalid metadata in pcache_meta_find_latest() Li Chen
2025-11-10 11:18   ` Dongsheng Yang
2025-11-10 12:32     ` Li Chen
2025-11-05  8:46 ` [RFC PATCH 0/2] Add cleanup_plugin for detecting problematic cleanup patterns Li Chen
2025-11-05  9:04   ` Li Chen
2025-11-05  9:49     ` Peter Zijlstra
2025-11-05 14:52       ` Li Chen
2025-11-05  8:46 ` [RFC PATCH 1/2] gcc-plugins: add cleanup_plugin for uninitialized cleanup detection Li Chen
2025-11-05  8:46 ` Li Chen [this message]
2025-11-05 12:41 ` [PATCH 0/3] dm-pcache: built-in support and metadata hardening Li Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251105084733.3598704-7-me@linux.beauty \
    --to=me@linux.beauty \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=nicolas.schier@linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox