[lkml]   [2014]   [Feb]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH] Change task_struct->comm to use RCU.
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).

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.

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);


Remaining problem:

Although the proposed solution will prevent the caller from emitting the
unexpected '\0' byte and the garbage bytes, memcpy(tmp, task->comm, 16) in
the proposed solution is not atomic. That is, "%pT" does not emit the '\0'
byte like "Penguin\0nux" but "%pT" still might emit "Penguininux".

To fix this problem, I proposed protecting memcpy(tmp, task->comm, 16) part
using RCU. This patch is a design for how the update side of task->comm will
look like if we use RCU approach.

Of course, this approach depends on that nobody prefers the speed of reading
task->comm over the atomicity of reading task->comm . If somebody strongly
objects on the cost of calling rcu_read_lock()/rcu_read_unlock() for the
atomicity, I'm fine without this patch.

 \ /
  Last update: 2014-02-25 14:41    [W:0.104 / U:4.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site