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

[Wang Liang <wangliang74@huawei.com>]

在 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@huawei.com>
>> ---
>>   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没必要吧


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

>> +
>>   	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;
> 同上
>
>> +
>>   	torture_cleanup_end();
>>   }
>>