| 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
| ||
|
Message-ID: <aG0qLaeAoTGaRs0n@pathway.suse.cz>
Date: Tue, 8 Jul 2025 16:24:45 +0200
From: Petr Mladek <pmladek@...e.com>
To: Nathan Chancellor <nathan@...nel.org>
Cc: Thomas Weißschuh <thomas.weissschuh@...utronix.de>,
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>,
Arnd Bergmann <arnd@...db.de>, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH 2/3] printk: kunit: support offstack cpumask
On Wed 2025-07-02 13:28:35, Nathan Chancellor wrote:
> On Wed, Jul 02, 2025 at 11:51:56AM +0200, Petr Mladek wrote:
> > From: Arnd Bergmann <arnd@...db.de>
> >
> > For large values of CONFIG_NR_CPUS, the newly added kunit test fails
> > to build:
> >
> > kernel/printk/printk_ringbuffer_kunit_test.c: In function 'test_readerwriter':
> > kernel/printk/printk_ringbuffer_kunit_test.c:279:1: error: the frame size of 1432 bytes is larger than 1280 bytes [-Werror=frame-larger-than=]
> >
> > Change this to use cpumask_var_t and allocate it dynamically when
> > CONFIG_CPUMASK_OFFSTACK is set.
> >
> > Fixes: 5ea2bcdfbf46 ("printk: ringbuffer: Add KUnit test")
> > Signed-off-by: Arnd Bergmann <arnd@...db.de>
> > [pmladek@...e.com: Correctly handle allocation failures and freeing using KUnit test API.]
> > Signed-off-by: Petr Mladek <pmladek@...e.com>
> > ---
> > kernel/printk/printk_ringbuffer_kunit_test.c | 18 ++++++++++++------
> > 1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/kernel/printk/printk_ringbuffer_kunit_test.c b/kernel/printk/printk_ringbuffer_kunit_test.c
> > index 217dcc14670c..0c3030fde8c2 100644
> > --- a/kernel/printk/printk_ringbuffer_kunit_test.c
> > +++ b/kernel/printk/printk_ringbuffer_kunit_test.c
> > @@ -216,6 +216,7 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
> > return 0;
> > }
> >
> > +KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
>
> This appears to break the build for me when CONFIG_CPUMASK_OFFSTACK is not
> set, like when enabling this test on top of x86_64 defconfig:
>
> In file included from kernel/printk/printk_ringbuffer_kunit_test.c:14:
> kernel/printk/printk_ringbuffer_kunit_test.c: In function 'prbtest_cpumask_cleanup':
> include/kunit/resource.h:409:32: error: cast specifies array type
> 409 | arg_type arg = (arg_type)in; \
> | ^
> kernel/printk/printk_ringbuffer_kunit_test.c:226:1: note: in expansion of macro 'KUNIT_DEFINE_ACTION_WRAPPER'
> 226 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Clang's error might be a little clearer with the "aka" note it provides.
>
> kernel/printk/printk_ringbuffer_kunit_test.c:226:1: error: used type 'cpumask_var_t' (aka 'struct cpumask[1]') where arithmetic or pointer type is required
> 226 | KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> include/kunit/resource.h:409:18: note: expanded from macro 'KUNIT_DEFINE_ACTION_WRAPPER'
> 409 | arg_type arg = (arg_type)in; \
> | ^ ~~
>
> > KUNIT_DEFINE_ACTION_WRAPPER(prbtest_kthread_cleanup, kthread_stop, struct task_struct *);
> >
> > static void prbtest_add_kthread_cleanup(struct kunit *test, struct task_struct *kthread)
Thanks a lot for the nice report.
The problem is how cpumask_var_t is defined in include/linux/cpumask_types.h:
#ifdef CONFIG_CPUMASK_OFFSTACK
typedef struct cpumask *cpumask_var_t;
#else
typedef struct cpumask cpumask_var_t[1];
#endif /* CONFIG_CPUMASK_OFFSTACK */
And KUNIT_DEFINE_ACTION_WRAPPER() expect that the 3rd parameter
is a pointer.
I am going to solve this by adding a wrapper over free_cpumask_var()
which would work with a pointer to cpumask_var_t.
It has another catch, though. It seems that the automatic clean up is
done after test_readerwriter() finishes. Therefore the variable
@test_cpus can't be on stack.
Anyway, the following seems to work with both CONFIG_CPUMASK_OFFSTACK
enabled and disabled:
--- a/kernel/printk/printk_ringbuffer_kunit_test.c
+++ b/kernel/printk/printk_ringbuffer_kunit_test.c
@@ -223,7 +223,17 @@ static int prbtest_reader(struct prbtest_data *test_data, unsigned long timeout_
return 0;
}
-KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, free_cpumask_var, cpumask_var_t);
+/*
+ * Add a custom wrapper around free_cpumask_var() to be used by
+ * KUNIT_DEFINE_ACTION_WRAPPER(). It allows to pass @mask using
+ * a pointer even when CONFIG_CPUMASK_OFFSTACK is disabled.
+ */
+static void prbtest_free_cpumask_var(cpumask_var_t *mask)
+{
+ free_cpumask_var(*mask);
+}
+
+KUNIT_DEFINE_ACTION_WRAPPER(prbtest_cpumask_cleanup, prbtest_free_cpumask_var, cpumask_var_t *);
KUNIT_DEFINE_ACTION_WRAPPER(prbtest_kthread_cleanup, kthread_stop, struct task_struct *);
static void prbtest_add_kthread_cleanup(struct kunit *test, struct task_struct *kthread)
@@ -244,16 +254,19 @@ static void test_readerwriter(struct kunit *test)
{
/* Equivalent to CONFIG_LOG_BUF_SHIFT=13 */
DEFINE_PRINTKRB(test_rb, 8, 5);
-
+ /*
+ * @test_cpus can't be on stack. THe pointer to this variable is passed
+ * to an automatic clean up action, see prbtest_free_cpumask_var().
+ */
+ static cpumask_var_t test_cpus;
struct prbtest_thread_data *thread_data;
struct prbtest_data *test_data;
struct task_struct *thread;
- cpumask_var_t test_cpus;
int cpu, reader_cpu;
int err;
KUNIT_ASSERT_TRUE(test, alloc_cpumask_var(&test_cpus, GFP_KERNEL));
- err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, test_cpus);
+ err = kunit_add_action_or_reset(test, prbtest_cpumask_cleanup, &test_cpus);
KUNIT_ASSERT_EQ(test, err, 0);
cpus_read_lock();
Powered by blists - more mailing lists