linux-kernel - Re: [PATCH 03/11] kthread: Pass in the thread's name during creation

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: <74a17cb6-9dcd-749b-5d06-b2c364d36ae2@oracle.com>
Date:   Sat, 11 Mar 2023 10:11:26 -0600
From:   michael.christie@...cle.com
To:     Christian Brauner <brauner@...nel.org>
Cc:     hch@...radead.org, stefanha@...hat.com, jasowang@...hat.com,
        mst@...hat.com, sgarzare@...hat.com,
        virtualization@...ts.linux-foundation.org, ebiederm@...ssion.com,
        torvalds@...ux-foundation.org, konrad.wilk@...cle.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 03/11] kthread: Pass in the thread's name during creation

On 3/11/23 2:53 AM, Christian Brauner wrote:
> On Fri, Mar 10, 2023 at 04:03:24PM -0600, Mike Christie wrote:
>> This has us pass in the thread's name during creation in kernel_thread.
>>
>> Signed-off-by: Mike Christie <michael.christie@...cle.com>
>> ---
>>  kernel/kthread.c | 35 ++++++++++++++---------------------
>>  1 file changed, 14 insertions(+), 21 deletions(-)
>>
>> diff --git a/kernel/kthread.c b/kernel/kthread.c
>> index 63574cee925e..831a55b406d8 100644
>> --- a/kernel/kthread.c
>> +++ b/kernel/kthread.c
>> @@ -38,6 +38,7 @@ struct task_struct *kthreadd_task;
>>  struct kthread_create_info
>>  {
>>  	/* Information passed to kthread() from kthreadd. */
>> +	char *full_name;
>>  	int (*threadfn)(void *data);
>>  	void *data;
>>  	int node;
>> @@ -343,10 +344,15 @@ static int kthread(void *_create)
>>  	/* Release the structure when caller killed by a fatal signal. */
>>  	done = xchg(&create->done, NULL);
>>  	if (!done) {
>> +		kfree(create->full_name);
>>  		kfree(create);
>>  		kthread_exit(-EINTR);
>>  	}
>>  
>> +	if (strlen(create->full_name) >= TASK_COMM_LEN)
>> +		self->full_name = create->full_name;
>> +	else
>> +		kfree(create->full_name);
> 
> This is monir but wwiw, this looks suspicious when reading it without
> more context. It took me a while to see that kthread->full_name is
> intended to store the untruncated name only if truncation actually needs
> to happen. So either we should always initialize this or we should add a
> comment. You can just send a tiny patch that I can fold into this one so
> you don't have to resend the whole series...
Ok. Thanks. I think always initializing it would make the code a lot easier
to understand and be nicer.

I don't think it would be too much extra memory used, and we don't have
to worry about some random userspace app breaking because it wanted to
configure the thread but the name got truncated.