| 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
| ||
|
Date: Wed, 22 Feb 2012 13:48:08 +0100 (CET)
From: Jan Engelhardt <jengelh@...ozas.de>
To: Andrew Morton <akpm@...ux-foundation.org>
cc: linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] treewide: fix memory corruptions when TASK_COMM_LEN
!= 16
On Wednesday 2012-02-01 04:01, Andrew Morton wrote:
>>
>> Ah yes, indeed. My reason for augmenting the size of t->comm was so
>> that `ps afx` could show a more complete name of certain kernel
>> threads' names. In this case, the kernel delivers the name via
>> procfs via seq_printf("%s, t->comm),
>
>Where does procfs do this?
In the handler for /proc/n/stat and status.
Meanwhile I have approached this from a different angle by using a
union. (And then I discovered that even psutils has a dumb 16-byte
buffer as well such that output is truncated -.- but that's another
story.)
parent b01543dfe67bb1d191998e90d20534dc354de059 (v3.3-rc4)
commit 726ff573fe3f2722ec7bd765b84750ceda5e518e
Author: Jan Engelhardt <jengelh@...ozas.de>
Date: Tue Feb 21 06:21:38 2012 +0100
task: provide a larger task command buffer
---
fs/exec.c | 16 +++++++++++++---
fs/proc/array.c | 8 ++++----
include/linux/binfmts.h | 5 ++++-
include/linux/sched.h | 20 ++++++++++++++++----
kernel/exit.c | 2 +-
kernel/kthread.c | 4 ++--
6 files changed, 40 insertions(+), 15 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 92ce83a..1191a52 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1052,6 +1052,15 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
}
EXPORT_SYMBOL_GPL(get_task_comm);
+char *get_task_comm_ex(char *buf, size_t z, struct task_struct *tsk)
+{
+ task_lock(tsk);
+ strlcpy(buf, tsk->comm_ex, z);
+ task_unlock(tsk);
+ return buf;
+}
+EXPORT_SYMBOL_GPL(get_task_comm_ex);
+
void set_task_comm(struct task_struct *tsk, char *buf)
{
task_lock(tsk);
@@ -1064,9 +1073,9 @@ void set_task_comm(struct task_struct *tsk, char *buf)
* Readers without a lock may see incomplete new
* names but are safe from non-terminating string reads.
*/
- memset(tsk->comm, 0, TASK_COMM_LEN);
+ memset(tsk->comm_ex, 0, sizeof(tsk->comm_ex));
wmb();
- strlcpy(tsk->comm, buf, sizeof(tsk->comm));
+ strlcpy(tsk->comm_ex, buf, sizeof(tsk->comm_ex));
task_unlock(tsk);
perf_event_comm(tsk);
}
@@ -1100,7 +1109,8 @@ int flush_old_exec(struct linux_binprm * bprm)
set_mm_exe_file(bprm->mm, bprm->file);
- filename_to_taskname(bprm->tcomm, bprm->filename, sizeof(bprm->tcomm));
+ filename_to_taskname(bprm->tcomm_ex, bprm->filename,
+ sizeof(bprm->tcomm_ex));
/*
* Release all of the old mmap stuff
*/
diff --git a/fs/proc/array.c b/fs/proc/array.c
index c602b8d..4bdbeb0 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -91,9 +91,9 @@ static inline void task_name(struct seq_file *m, struct task_struct *p)
int i;
char *buf, *end;
char *name;
- char tcomm[sizeof(p->comm)];
+ char tcomm[sizeof(p->comm_ex)];
- get_task_comm(tcomm, p);
+ get_task_comm_ex(tcomm, sizeof(tcomm), p);
seq_puts(m, "Name:\t");
end = m->buf + m->size;
@@ -375,7 +375,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
cputime_t cutime, cstime, utime, stime;
cputime_t cgtime, gtime;
unsigned long rsslim = 0;
- char tcomm[sizeof(task->comm)];
+ char tcomm[sizeof(task->comm_ex)];
unsigned long flags;
state = *get_task_state(task);
@@ -390,7 +390,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
}
}
- get_task_comm(tcomm, task);
+ get_task_comm_ex(tcomm, sizeof(tcomm), task);
sigemptyset(&sigign);
sigemptyset(&sigcatch);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index 0092102..a19d1d1 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -58,7 +58,10 @@ struct linux_binprm {
unsigned interp_flags;
unsigned interp_data;
unsigned long loader, exec;
- char tcomm[TASK_COMM_LEN];
+ union {
+ char tcomm[TASK_COMM_LEN];
+ char tcomm_ex[32];
+ };
};
#define BINPRM_FLAGS_ENFORCE_NONDUMP_BIT 0
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 7d379a6..977eced 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1378,10 +1378,21 @@ struct task_struct {
* credentials (COW) */
struct cred *replacement_session_keyring; /* for KEYCTL_SESSION_TO_PARENT */
- char comm[TASK_COMM_LEN]; /* executable name excluding path
- - access with [gs]et_task_comm (which lock
- it with task_lock())
- - initialized normally by setup_new_exec */
+ union {
+ /*
+ * Executable name excluding path. Access it with
+ * [gs]et_task_comm (which lock it with task_lock()).
+ * Initialized normally by setup_new_exec.
+ */
+ char comm[TASK_COMM_LEN];
+ /*
+ * The .comm member is kept, while we try to identify the codes
+ * that unfortunately rely on its exact size of 16. Apparently
+ * not all those sites have been spotted in previous attempts
+ * of mine to enlarge comm.
+ */
+ char comm_ex[32];
+ };
/* file system info */
int link_count, total_link_count;
#ifdef CONFIG_SYSVIPC
@@ -2295,6 +2306,7 @@ 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);
+extern char *get_task_comm_ex(char *to, size_t z, struct task_struct *tsk);
#ifdef CONFIG_SMP
void scheduler_ipi(void);
diff --git a/kernel/exit.c b/kernel/exit.c
index 4b4042f..2c9569a 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -414,7 +414,7 @@ void daemonize(const char *name, ...)
sigset_t blocked;
va_start(args, name);
- vsnprintf(current->comm, sizeof(current->comm), name, args);
+ vsnprintf(current->comm_ex, sizeof(current->comm_ex), name, args);
va_end(args);
/*
diff --git a/kernel/kthread.c b/kernel/kthread.c
index 3d3de63..09dba54 100644
--- a/kernel/kthread.c
+++ b/kernel/kthread.c
@@ -196,8 +196,8 @@ struct task_struct *kthread_create_on_node(int (*threadfn)(void *data),
va_list args;
va_start(args, namefmt);
- vsnprintf(create.result->comm, sizeof(create.result->comm),
- namefmt, args);
+ vsnprintf(create.result->comm_ex,
+ sizeof(create.result->comm_ex), namefmt, args);
va_end(args);
/*
* root may have changed our (kthreadd's) priority or CPU mask.
--
# Created with git-export-patch
--
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/
Powered by blists - more mailing lists