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

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: <20180723124154.14ef0679@gandalf.local.home>
Date:   Mon, 23 Jul 2018 12:41:54 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Snild Dolkow <snild@...y.com>
Cc:     <linux-kernel@...r.kernel.org>, Ingo Molnar <mingo@...nel.org>,
        Jens Axboe <axboe@...nel.dk>, Tejun Heo <tj@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Peter Enderborg <peter.enderborg@...y.com>,
        Yoshitaka Seto <yoshitaka.seto@...y.com>,
        Oleksiy Avramchenko <oleksiy.avramchenko@...y.com>,
        KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
        John Stultz <john.stultz@...aro.org>
Subject: Re: [PATCH RESEND] kthread, tracing: Don't expose half-written comm
 when creating kthreads

On Mon, 23 Jul 2018 17:49:36 +0200
Snild Dolkow <snild@...y.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().