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

From: michael . christie
Date: Sat Mar 11 2023 - 11:11:56 EST


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@xxxxxxxxxx>
>> ---
>> 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.