[PATCH] sanitize task->comm to avoid leaking escape codes

[Kees Cook]

Through get_task_comm() and many direct uses of task->comm in the kernel,
it is possible for escape codes and other non-printables to leak into
dmesg, syslog, etc.  In the worst case, these strings could be used to
attack administrators using vulnerable terminal emulators, and at least
cause confusion through the injection of \r characters.

This patch sanitizes task->comm to only contain printable characters
when it is set.  Additionally, it redefines get_task_comm so that it
cannot be misused by callers (presently nothing was incorrectly calling
get_task_comm's unsafe use of strncpy).

Signed-off-by: Kees Cook <kees.cook@xxxxxxxxxxxxx>
---
 fs/exec.c             |   17 +++++++++++++----
 include/linux/sched.h |    3 ++-
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 85092e3..aa84f66 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -935,17 +935,18 @@ static void flush_old_files(struct files_struct * files)
 	spin_unlock(&files->file_lock);
 }
 
-char *get_task_comm(char *buf, struct task_struct *tsk)
+char *get_task_comm_size(char *buf, size_t len, struct task_struct *tsk)
 {
-	/* buf must be at least sizeof(tsk->comm) in size */
 	task_lock(tsk);
-	strncpy(buf, tsk->comm, sizeof(tsk->comm));
+	strlcpy(buf, tsk->comm, len);
 	task_unlock(tsk);
 	return buf;
 }
 
 void set_task_comm(struct task_struct *tsk, char *buf)
 {
+	size_t i;
+
 	task_lock(tsk);
 
 	/*
@@ -956,7 +957,15 @@ void set_task_comm(struct task_struct *tsk, char *buf)
 	 */
 	memset(tsk->comm, 0, TASK_COMM_LEN);
 	wmb();
-	strlcpy(tsk->comm, buf, sizeof(tsk->comm));
+
+	/* sanitize non-printable characters */
+	for (i = 0; buf[i] && i < (sizeof(tsk->comm) - 1); i++) {
+		if (!isprint(buf[i]))
+			tsk->comm[i] = '?';
+		else
+			tsk->comm[i] = buf[i];
+	}
+
 	task_unlock(tsk);
 	perf_event_comm(tsk);
 }
diff --git a/include/linux/sched.h b/include/linux/sched.h
index f118809..0ba69dc 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2117,7 +2117,8 @@ extern long do_fork(unsigned long, unsigned long, struct pt_regs *, unsigned lon
 struct task_struct *fork_idle(int);
 
 extern void set_task_comm(struct task_struct *tsk, char *from);
-extern char *get_task_comm(char *to, struct task_struct *tsk);
+#define get_task_comm(buf, task) get_task_comm_size(buf, sizeof(buf), task)
+extern char *get_task_comm_size(char *to, size_t len, struct task_struct *tsk);
 
 #ifdef CONFIG_SMP
 extern unsigned long wait_task_inactive(struct task_struct *, long match_state);
-- 
1.7.1


-- 
Kees Cook
Ubuntu Security Team
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/