linux-kernel - Re: [PATCH 2/3] printk: kunit: support offstack cpumask

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <85b5d75c-8c89-4687-b3ac-3041c1f109be@app.fastmail.com>
Date: Thu, 10 Jul 2025 16:08:25 +0200
From: "Arnd Bergmann" <arnd@...db.de>
To: "Petr Mladek" <pmladek@...e.com>,
 Thomas Weißschuh <thomas.weissschuh@...utronix.de>
Cc: "Nathan Chancellor" <nathan@...nel.org>,
 "John Ogness" <john.ogness@...utronix.de>,
 "Dan Carpenter" <dan.carpenter@...aro.org>,
 "Steven Rostedt" <rostedt@...dmis.org>,
 "Sergey Senozhatsky" <senozhatsky@...omium.org>,
 "Kees Cook" <kees@...nel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
 "David Gow" <davidgow@...gle.com>, "Arnd Bergmann" <arnd@...nel.org>,
 linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH 2/3] printk: kunit: support offstack cpumask

On Thu, Jul 10, 2025, at 15:51, Petr Mladek wrote:
> On Wed 2025-07-09 14:53:29, Thomas Weißschuh wrote:
>> On Wed, Jul 09, 2025 at 01:36:14PM +0200, Petr Mladek wrote:
>> if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {
>> 	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
>> 	KUNIT_ASSERT_EQ(test, err, 0);
>> }
>
> It is likely a matter of taste but I like this idea. It looks better
> than passing an invalid pointer and hope nobody would do anything
> with it.
>
> The only problem is that
>
>     if (IS_ENABLED(CONFIG_CPUMASK_OFFSTACK)) {
>
> did not prevented the compiler warning. I guess that the code was still
> compiled and later just optimized out.

Right, gcc does some of the warnings after dead code eliminations
and some before. clang tries to do all warnings before eliminating
dead code, so you still lose.

> /*
>  * A cast would be needed for the clean up action when the cpumask was 
> on stack.
>  * Also it would leak the stack address to the cleanup thread.
>  * And alloc_cpu_mask() and free_cpumask_var() would do nothing anyway.
>  */
> #ifdef CONFIG_CPUMASK_OFFSTACK
> KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, 
> cpumask_var_t);
>
> static void prbtest_alloc_cpumask(struct kunit *test, cpumask_var_t *mask)
> {
> 	int err;
>
> 	KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(mask, GFP_KERNEL));
> 	err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, *mask);
> 	KUNIT_ASSERT_EQ(test, err, 0);
> }
> #else
> static inline
> void prbtest_alloc_cpumask(struct kunit *test, cpumask_var_t *mask) {}
> #endif
>
> which will be called in test_readerwriter().

Looks fine to me

> It seems to work, ..., sigh. I did not expect so many troubles with
> a tiny detail.

I wonder if just making the cpumask_t 'static' would still be
simpler, given that there are no concurrent callers.

    Arnd