linux-kernel - Re: [RFC PATCH] workqueue: Fix rescuer task's name truncated

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] [day] [month] [year] [list]

Message-ID: <185e1d7a-719d-4fec-8e4f-e7cf0bea1573@huawei.com>
Date: Tue, 14 May 2024 09:37:04 +0800
From: Wenchao Hao <haowenchao2@...wei.com>
To: Tejun Heo <tj@...nel.org>, Aaron Tomlin <atomlin@...mlin.com>
CC: Lai Jiangshan <jiangshanlai@...il.com>, <linux-kernel@...r.kernel.org>,
	Wenchao Hao <haowenchao22@...il.com>, <neelx@...hat.com>
Subject: Re: [RFC PATCH] workqueue: Fix rescuer task's name truncated

On 2024/5/14 1:17, Tejun Heo wrote:
> Hello
> 
> On Sat, May 11, 2024 at 03:15:11PM +0100, Aaron Tomlin wrote:
>>> @@ -5439,6 +5439,8 @@ static int init_rescuer(struct workqueue_struct *wq)
>>>   		return ret;
>>>   	}
>>>   
>>> +	snprintf(rescuer->desc, sizeof(rescuer->desc), "%s", wq->name);
> 
> Can you please address the testbot reported warning?
> 

Of course.

The warning is also reported here: https://lore.kernel.org/all/20240513030639.3772468-1-haowenchao2@huawei.com/

It's caused because commit 31c89007285d ("workqueue.c: Increase workqueue name length") increase
WQ_NAME_LEN from 24 to 32, but forget to increase WORKER_DESC_LEN which should equalto WQ_NAME_LEN.
Same usage can be found in process_one_work(), it called strscpy() which would not WARNING.

I sent a V2 patch with following changes appended for this warning, maybe you missed them.

diff --git a/include/linux/workqueue.h b/include/linux/workqueue.h
index 158784dd189a..72031fa80414 100644
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -92,7 +92,7 @@ enum wq_misc_consts {
         WORK_BUSY_RUNNING       = 1 << 1,
  
         /* maximum string length for set_worker_desc() */
-       WORKER_DESC_LEN         = 24,
+       WORKER_DESC_LEN         = 32,
  };
  
  /* Convenience constants - of type 'unsigned long', not 'enum'! */

>>>   	wq->rescuer = rescuer;
>>>   	if (wq->flags & WQ_UNBOUND)
>>>   		kthread_bind_mask(rescuer->task, wq_unbound_cpumask);
>>> @@ -6289,6 +6291,8 @@ void wq_worker_comm(char *buf, size_t size, struct task_struct *task)
>>>   						  worker->desc);
>>>   			}
>>>   			raw_spin_unlock_irq(&pool->lock);
>>> +		} else if (worker->desc[0] != '\0') {
>>> +			scnprintf(buf + off, size - off, "-%s", worker->desc);
>>>   		}
>>>   	}
>>>   
>>> -- 
>>
>> Reviewed-by: Aaron Tomlin <atomlin@...mlin.com>
> 
> The patch looks fine to me otherwise.
> 
> Thanks.
>