linux-kernel - Re: [PATCH] Change task

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: <20140225144643.GU9987@twins.programming.kicks-ass.net>
Date:	Tue, 25 Feb 2014 15:46:43 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
Cc:	laijs@...fujitsu.com, akpm@...ux-foundation.org, joe@...ches.com,
	keescook@...omium.org, geert@...ux-m68k.org, jkosina@...e.cz,
	viro@...iv.linux.org.uk, davem@...emloft.net,
	linux-kernel@...r.kernel.org, mingo@...e.hu, rostedt@...dmis.org,
	tglx@...utronix.de
Subject: Re: [PATCH] Change task_struct->comm to use RCU.

On Tue, Feb 25, 2014 at 09:54:01PM +0900, Tetsuo Handa wrote:
> Lai Jiangshan wrote:
> > CC scheduler people.
> > 
> > I can't figure out what we get with this patch.
> > 
> OK. Welcome to this thread. I'll explain you what is going on.
> 
> Current problem:
> 
>   printk("%s\n", task->comm) is racy because "%s" format specifier assumes that
>   the corresponding argument does not change between strnlen() and the for loop
>   at string() in lib/vsnprintf.c . If task->comm was "Hello Linux" until
>   strnlen() and becomes "Penguin" before the for loop, "%s" will emit
>   "Penguin\0nux" (note the unexpected '\0' byte and the garbage bytes).

I would have actually expected it to stop emitting chars at \0. But
sure. Couldn't care less though; that's what you get, we all know this,
we've all been through this discussion several times. Get over it
already.

One of the last threads on this is:

  https://lkml.org/lkml/2011/5/17/516

>   Likewise, audit_log_untrustedstring(ab, current->comm) is racy.
>   If task->comm was "Hello Linux" until audit_string_contains_control() in
>   audit_log_n_untrustedstring() returns false, and becomes "Penguin" before
>   memcpy() in audit_log_n_string() is called, memcpy() will emit "Penguin\0nux"
>   into the audit log, which results in loss of information (e.g. SELinux
>   context) due to the unexpected '\0' byte.

I expect the audit people don't like this? Also, how do audit and the
LSM crap things interact? I thought they were both different piles of
ignorable goo?

See there's not actually a problem statement here at all, so you can't
go about proposing solutions quite yet.

> Proposed solution:
> 
>   To fix abovementioned problem, I proposed commcpy() and "%pT" format
>   specifier which does
> 
>     char tmp[16];
>     memcpy(tmp, task->comm, 16);
>     tmp[15] = '\0';
>     sprintf(buf, "%s", tmp);
> 
>   instead of
> 
>     sprintf(buf, "%s", task->comm);
> 
>   .

How about you do what you're supposed to do when you want a reliable
->comm and use get_task_comm()?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/