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
next prev 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