[PATCH 0/5] malloc: add __free() cleanup handlers

[PATCH 0/5] malloc: add __free() cleanup handlers

[Ahmad Fatoum]

The cleanup infrastructure was added to barebox a while back in
commit 332e3542e5ed ("Port Linux __cleanup() based guard
infrastructure"), but without any DEFINE_FREE() handlers, so nothing
could be passed to __free() so far.

Let's start with malloc and kmalloc, so buffers can be tied to the
scope they are used in:

	void *buf __free(free)  =  malloc(size);
	void *foo __free(kfree) = kzalloc(size, GFP_KERNEL);

[k]free_sensitive() looks up the usable size of the buffer before
zeroing it, so it gets the same test rather than just a NULL check
to avoid an error pointer dereference inside the allocator.

Ahmad Fatoum (5):
  commands: rksecure: exit if invalid integer passed to -b
  firmware-zynqmp: don't ignore the kstrtouint() result in parse_reg()
  include: compiler: make __must_check a yes-op
  malloc: add __free() cleanup handlers
  test: self: add cleanup selftest

 arch/arm/mach-zynqmp/firmware-zynqmp.c |  6 +-
 commands/rksecure.c                    |  4 +-
 include/linux/compiler_types.h         |  4 --
 include/linux/slab.h                   |  5 ++
 include/malloc.h                       |  5 ++
 test/self/Kconfig                      | 10 +++
 test/self/Makefile                     |  1 +
 test/self/cleanup.c                    | 89 ++++++++++++++++++++++++++
 8 files changed, 118 insertions(+), 6 deletions(-)
 create mode 100644 test/self/cleanup.c

-- 
2.47.3

[PATCH 1/5] commands: rksecure: exit if invalid integer passed to -b

[Ahmad Fatoum]

kstrtouint() is defined with __must_check and we will enforce this in a
follow-up commit. The rksecure command used the function, but didn't check
its return value. Fix this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 commands/rksecure.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/commands/rksecure.c b/commands/rksecure.c
index d761cac152ce..8f277af761dc 100644
--- a/commands/rksecure.c
+++ b/commands/rksecure.c
@@ -93,7 +93,9 @@ static int do_rksecure(int argc, char *argv[])
 			hash = optarg;
 			break;
 		case 'b':
-			kstrtouint(optarg, 10, &key_size_bits);
+			ret = kstrtouint(optarg, 10, &key_size_bits);
+			if (ret)
+				return ret;
 			break;
 		case 'l':
 			lockdown = 1;
-- 
2.47.3

[PATCH 2/5] firmware-zynqmp: don't ignore the kstrtouint() result in parse_reg()

[Ahmad Fatoum]

parse_reg() decodes the index out of the ggs%u/pggs%u parameter names the
driver registers itself, so the conversion can't really fail.

We define __must_check as empty currently, so the __must_check on
kstrtouint() doesn't catch this, but it will once we change the
definition, so prepare for that.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-zynqmp/firmware-zynqmp.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-zynqmp/firmware-zynqmp.c b/arch/arm/mach-zynqmp/firmware-zynqmp.c
index 039a46e76759..d21e2c1137f1 100644
--- a/arch/arm/mach-zynqmp/firmware-zynqmp.c
+++ b/arch/arm/mach-zynqmp/firmware-zynqmp.c
@@ -712,7 +712,11 @@ EXPORT_SYMBOL_GPL(zynqmp_pm_get_eemi_ops);
 static bool parse_reg(const char *reg, unsigned *idx)
 {
 	bool pggs = reg[0] == 'p';
-	kstrtouint(reg + pggs + sizeof("ggs") - 1, 10, idx);
+
+	/* the names are registered by this driver as ggs%u and pggs%u */
+	if (WARN_ON(kstrtouint(reg + pggs + sizeof("ggs") - 1, 10, idx)))
+		*idx = 0;
+
 	return pggs;
 }
 
-- 
2.47.3

[PATCH 3/5] include: compiler: make __must_check a yes-op

[Ahmad Fatoum]

__must_check is gated behind CONFIG_ENABLE_MUST_CHECK, which we never
define.

Linux removed the option in 1967939462641 ("Compiler Attributes: remove
CONFIG_ENABLE_MUST_CHECK"), so let's do the same.

CI build with -Werror is clean now with it enabled.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/compiler_types.h | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 798c2e637daa..0e49c6795409 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -357,11 +357,7 @@ struct ftrace_likely_data {
 #endif
 
 
-#ifdef CONFIG_ENABLE_MUST_CHECK
 #define __must_check		__attribute__((warn_unused_result))
-#else
-#define __must_check
-#endif
 
 #if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
 #define notrace			__attribute__((hotpatch(0, 0)))
-- 
2.47.3

[PATCH 4/5] malloc: add __free() cleanup handlers

[Ahmad Fatoum]

The cleanup infrastructure was added to barebox a while back in
commit 332e3542e5ed ("Port Linux __cleanup() based guard
infrastructure"), but without any DEFINE_FREE() handlers, so nothing
could be passed to __free() so far.

Let's start with malloc and kmalloc, so buffers can be tied to the
scope they are used in:

	void *buf __free(free)  =  malloc(size);
	void *foo __free(kfree) = kzalloc(size, GFP_KERNEL);

[k]free_sensitive() looks up the usable size of the buffer before
zeroing it, so it gets the same test rather than just a NULL check
to avoid an error pointer dereference inside the allocator.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/linux/slab.h | 5 +++++
 include/malloc.h     | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 93ce25a58299..9ccb8c6a0101 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -4,6 +4,8 @@
 #define _LINUX_SLAB_H
 
 #include <dma.h>
+#include <linux/cleanup.h>
+#include <linux/err.h>
 #include <linux/overflow.h>
 #include <linux/string.h>
 #include <linux/gfp.h>
@@ -116,4 +118,7 @@ static inline char *kstrdup(const char *str, gfp_t flags)
 
 #define kstrdup_const(str, flags) strdup_const(str)
 
+DEFINE_FREE(kfree, void *, if (!IS_ERR_OR_NULL(_T)) kfree(_T))
+DEFINE_FREE(kfree_sensitive, void *, if (!IS_ERR_OR_NULL(_T)) kfree_sensitive(_T))
+
 #endif /* _LINUX_SLAB_H */
diff --git a/include/malloc.h b/include/malloc.h
index 82fa2bb39c8e..b7f1aaac95d2 100644
--- a/include/malloc.h
+++ b/include/malloc.h
@@ -2,7 +2,9 @@
 #ifndef __MALLOC_H
 #define __MALLOC_H
 
+#include <linux/cleanup.h>
 #include <linux/compiler.h>
+#include <linux/err.h>
 #include <types.h>
 
 #define MALLOC_SHIFT_MAX	30
@@ -81,6 +83,9 @@ static inline int mem_malloc_is_initialized(void)
 }
 #endif
 
+DEFINE_FREE(free, void *, if (!IS_ERR_OR_NULL(_T)) free(_T))
+DEFINE_FREE(free_sensitive, void *, if (!IS_ERR_OR_NULL(_T)) free_sensitive(_T))
+
 static inline bool want_init_on_alloc(void)
 {
 	return IS_ENABLED(CONFIG_INIT_ON_ALLOC_DEFAULT_ON);
-- 
2.47.3

[PATCH 5/5] test: self: add cleanup selftest

[Ahmad Fatoum]

The new __free() handlers have no user in the tree yet, so add a
selftest that exercises them: it checks that leaving the scope really
runs the cleanup, that NULL and error pointers are never passed to free(),
and that no_free_ptr() inhibits the cleanup.

Assisted-by: Claude:opus-5-1m
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/Kconfig   | 10 +++++
 test/self/Makefile  |  1 +
 test/self/cleanup.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 100 insertions(+)
 create mode 100644 test/self/cleanup.c

diff --git a/test/self/Kconfig b/test/self/Kconfig
index 85a3ef790116..804fe6ac8d31 100644
--- a/test/self/Kconfig
+++ b/test/self/Kconfig
@@ -32,6 +32,7 @@ config SELFTEST_ENABLE_ALL
 	select SELFTEST_RANGE
 	select SELFTEST_PRINTF
 	select SELFTEST_MALLOC
+	select SELFTEST_CLEANUP
 	select SELFTEST_PROGRESS_NOTIFIER
 	select SELFTEST_OF_MANIPULATION
 	select SELFTEST_ENVIRONMENT_VARIABLES if ENVIRONMENT_VARIABLES
@@ -71,6 +72,15 @@ config SELFTEST_MALLOC
 	help
 	  Tests barebox memory allocator
 
+config SELFTEST_CLEANUP
+	bool "scope-based cleanup selftest"
+	help
+	  Tests the __free() cleanup handlers the barebox allocators define,
+	  i.e. that leaving a scope frees the buffer, that NULL and error
+	  pointers are never freed and that no_free_ptr() inhibits cleanup.
+
+	  If unsure, say n.
+
 config SELFTEST_TALLOC
 	bool "talloc() selftest"
 	help
diff --git a/test/self/Makefile b/test/self/Makefile
index 2bfdbb9949df..2fecab1f8ee3 100644
--- a/test/self/Makefile
+++ b/test/self/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_SELFTEST) += core.o
 obj-$(CONFIG_SELFTEST_BASE64) += base64.o
 obj-$(CONFIG_SELFTEST_RANGE) += range.o
 obj-$(CONFIG_SELFTEST_MALLOC) += malloc.o
+obj-$(CONFIG_SELFTEST_CLEANUP) += cleanup.o
 obj-$(CONFIG_SELFTEST_TALLOC) += talloc.o
 obj-$(CONFIG_SELFTEST_PRINTF) += printf.o
 CFLAGS_printf.o += -Wno-format-security -Wno-format
diff --git a/test/self/cleanup.c b/test/self/cleanup.c
new file mode 100644
index 000000000000..7a3b63cc897f
--- /dev/null
+++ b/test/self/cleanup.c
@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <common.h>
+#include <bselftest.h>
+#include <malloc.h>
+#include <linux/err.h>
+#include <linux/slab.h>
+
+BSELFTEST_GLOBALS();
+
+static void *freed[2];
+static int frees;
+static void count_free(void *mem)
+{
+	if (frees < ARRAY_SIZE(freed))
+		freed[frees] = mem;
+	frees++;
+	free(mem);
+}
+DEFINE_FREE(count_free, void *, if (!IS_ERR_OR_NULL(_T)) count_free(_T))
+
+static void test_cleanup(void)
+{
+	void *first, *second;
+
+	/* leaving the scope must free the buffer, and only then */
+	frees = 0;
+	{
+		void *p __free(count_free) = malloc(64);
+
+		assert_cond(p != NULL);
+		assert_cond(frees == 0);
+		first = p;
+	}
+	assert_cond(frees == 1);
+	assert_cond(freed[0] == first);
+	/* the variable defined last is freed first */
+	frees = 0;
+	{
+		void *p __free(count_free) = malloc(64);
+		void *q __free(count_free) = malloc(64);
+
+		first = p;
+		second = q;
+	}
+	assert_cond(frees == 2);
+	assert_cond(freed[0] == second);
+	assert_cond(freed[1] == first);
+
+	/* NULL must not reach the allocator */
+	{
+		void *p __free(free) = NULL;
+		void *q __free(free_sensitive) = NULL;
+		void *r __free(kfree) = NULL;
+		void *s __free(kfree_sensitive) = NULL;
+
+		assert_cond(p == NULL);
+		assert_cond(q == NULL);
+		assert_cond(r == NULL);
+		assert_cond(s == NULL);
+	}
+
+	/* and neither may error pointers */
+	{
+		void *p __free(free) = ERR_PTR(-EINVAL);
+		void *q __free(free_sensitive) = ERR_PTR(-EINVAL);
+		void *r __free(kfree) = ERR_PTR(-ENOMEM);
+		void *s __free(kfree_sensitive) = ERR_PTR(-ENOMEM);
+
+		assert_cond(IS_ERR(p));
+		assert_cond(IS_ERR(q));
+		assert_cond(IS_ERR(r));
+		assert_cond(IS_ERR(s));
+	}
+
+	/* no_free_ptr() inhibits the cleanup, so the buffer stays taken */
+	{
+		void *p __free(kfree) = kmalloc(64, GFP_KERNEL);
+		assert_cond(p != NULL);
+		first = no_free_ptr(p);
+	}
+	second = kmalloc(64, GFP_KERNEL);
+	assert_cond(second != first);
+	kfree(first);
+	kfree(second);
+}
+bselftest(core, test_cleanup);
-- 
2.47.3

Re: [PATCH 0/5] malloc: add __free() cleanup handlers

[Sascha Hauer]

On Tue, 22 Sep 2026 12:47:51 +0200, Ahmad Fatoum wrote:
> The cleanup infrastructure was added to barebox a while back in
> commit 332e3542e5ed ("Port Linux __cleanup() based guard
> infrastructure"), but without any DEFINE_FREE() handlers, so nothing
> could be passed to __free() so far.
> 
> Let's start with malloc and kmalloc, so buffers can be tied to the
> scope they are used in:
> 
> [...]

Applied, thanks!

[1/5] commands: rksecure: exit if invalid integer passed to -b
      https://git.pengutronix.de/cgit/barebox/commit/?id=4daf5bda08bb (link may not be stable)
[2/5] firmware-zynqmp: don't ignore the kstrtouint() result in parse_reg()
      https://git.pengutronix.de/cgit/barebox/commit/?id=654e1801d6fc (link may not be stable)
[3/5] include: compiler: make __must_check a yes-op
      https://git.pengutronix.de/cgit/barebox/commit/?id=46ba4067337e (link may not be stable)
[4/5] malloc: add __free() cleanup handlers
      https://git.pengutronix.de/cgit/barebox/commit/?id=ec28aeaedf8f (link may not be stable)
[5/5] test: self: add cleanup selftest
      https://git.pengutronix.de/cgit/barebox/commit/?id=b303ec45203e (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>