Re: [PATCH RESEND] kthread, tracing: Don't expose half-written comm when creating kthreads

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 23 Jul 2018 17:49:36 +0200
Snild Dolkow <snild@sony.com> wrote:

> On 07/23/2018 05:37 PM, Steven Rostedt wrote:

> Will add:
> 
> 		/*
> 		 * task is already visible to other tasks, so updating
> 		 * COMM must be protected.
> 		 */

Thanks.

> 
> Any issues with the commit message? Reading it back again now, it doesn't
> seem quite as clear as when I wrote it.

Yeah, I think it does need some updates:

> There was a window for racing when task->comm was being written. The

It would be nice to explain this race window in more detail.

> vsnprintf function writes 16 bytes, then counts the rest, then null
> terminates. In the meantime, other threads could see the non-terminated
> comm value. In our case, it got into the trace system's saved cmdlines
> and could cause stack corruption when strcpy'd out of there.

Perhaps add in the change log something about the fact that the
vsprintf() is performed on the COMM when the task is visible to other
tasks, and that could cause problems if other tasks read the COMM (like
in tracing) without updating it properly with set_task_comm().

-- Steve


> 
> The workaround in e09e28671 (use strlcpy in __trace_find_cmdline) was
> likely needed because of this bug.
> 
> Solved by vsnprintf:ing to a local buffer, then using set_task_comm().