Re: [PATCH] locktorture: Fix memory leak in param_set_cpumask()

From: Wang Liang

Date: Thu Sep 18 2025 - 07:17:50 EST

在 2025/9/18 17:03, Paul E. McKenney 写道:

On Mon, Sep 15, 2025 at 10:13:33AM +0800, Wang Liang wrote:

在 2025/9/12 10:16, Zhang Changzhong 写道:

在 2025/9/12 9:57, Wang Liang 写道:

When setting the locktorture module parameter 'bind_writers', the variable
'cpumask_var_t bind_writers' is allocated in param_set_cpumask(). But it
is not freed, when removing module or setting the parameter again.

Below kmemleak trace is seen for this issue:

unreferenced object 0xffff888100aabff8 (size 8):
comm "bash", pid 323, jiffies 4295059233
hex dump (first 8 bytes):
07 00 00 00 00 00 00 00 ........
backtrace (crc ac50919):
__kmalloc_node_noprof+0x2e5/0x420
alloc_cpumask_var_node+0x1f/0x30
param_set_cpumask+0x26/0xb0 [locktorture]
param_attr_store+0x93/0x100
module_attr_store+0x1b/0x30
kernfs_fop_write_iter+0x114/0x1b0
vfs_write+0x300/0x410
ksys_write+0x60/0xd0
do_syscall_64+0xa4/0x260
entry_SYSCALL_64_after_hwframe+0x77/0x7f

This issue can be reproduced by:
insmod locktorture.ko
echo 0-2 > /sys/module/locktorture/parameters/bind_writers
rmmod locktorture

or:
insmod locktorture.ko
echo 0-2 > /sys/module/locktorture/parameters/bind_writers
echo 0-2 > /sys/module/locktorture/parameters/bind_writers

The parameter 'bind_readers' also has the same problem. Free the memory
when removing module or setting the parameter.

Fixes: 73e341242483 ("locktorture: Add readers_bind and writers_bind module parameters")
Signed-off-by: Wang Liang <wangliang74@xxxxxxxxxx>
---
kernel/locking/locktorture.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/kernel/locking/locktorture.c b/kernel/locking/locktorture.c
index ce0362f0a871..cad80c050502 100644
--- a/kernel/locking/locktorture.c
+++ b/kernel/locking/locktorture.c
@@ -70,6 +70,9 @@ static int param_set_cpumask(const char *val, const struct kernel_param *kp)
int ret;
char *s;
+ free_cpumask_var(*cm_bind);
+ *cm_bind = NULL;

这个NULL没必要吧

Assuming this translates to "This NULL is unnecessary", I have to
agree with Zhang Changzhong. I would go further and argue that the
free_cpumask_var() is also unnecessary here.

Thanks for your replies!

The free_cpumask_var() in param_set_cpumask() may be necessary(). If user
set the parameter for two times, the pointer will point to a new memory,
and no one hold the old memory, which trigger a memory leak.

Setting global pointer to NULL after free may be more safe. ^-^

In lock_torture_cleanup(), you mean? I would agree with that.

+
if (!alloc_cpumask_var(cm_bind, GFP_KERNEL)) {
s = "Out of memory";
ret = -ENOMEM;
@@ -1211,6 +1214,12 @@ static void lock_torture_cleanup(void)
cxt.cur_ops->exit();
cxt.init_called = false;
}
+
+ free_cpumask_var(bind_readers);
+ free_cpumask_var(bind_writers);
+ bind_readers = NULL;
+ bind_writers = NULL;

同上

But here I agree with Wang Liang, as it helps people running debuggers
on the kernel. Instead of a dangling pointer, they see a NULL pointer.

Except... Is this NULLing really the right thing to do for
CONFIG_CPUMASK_OFFSTACK=n kernels?

Thanx, Paul

For CONFIG_CPUMASK_OFFSTACK=n, the NULLing may be not appropriate. I will remove it.

+
torture_cleanup_end();
}