[PATCH v2 1/3] coredump: use get_task_comm for %e filename format

[PATCH v2 1/3] coredump: use get_task_comm for %e filename format

[Jiri Slaby]

We currently access current->comm directly. As we have
prctl(PR_SET_NAME), we need the access be protected by task_lock. This
is exactly what get_task_comm does, so use it.

I'm not 100% convinced prctl(PR_SET_NAME) may be called at the time of
core dump, but the locking won't hurt. Note that siglock is not held
in format_corename.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 fs/exec.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 5ee7562..155c6d4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1656,9 +1656,12 @@ static int format_corename(struct core_name *cn, long signr)
 				up_read(&uts_sem);
 				break;
 			/* executable */
-			case 'e':
-				err = cn_printf(cn, "%s", current->comm);
+			case 'e': {
+				char comm[TASK_COMM_LEN];
+				err = cn_printf(cn, "%s",
+						get_task_comm(comm, current));
 				break;
+			}
 			case 'E':
 				err = cn_print_exe_file(cn);
 				break;
-- 
1.7.4.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v2 2/3] coredump: use task comm instead of (unknown)

[Jiri Slaby]

If we don't know the file corresponding to the binary (i.e. exe_file
is unknown), use "task->comm (path unknown)" instead of simple
"(unknown)" as suggested by ak.

The fallback is the same as %e except it will append "(path unknown)".

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Oleg Nesterov <oleg@redhat.com>
---
 fs/exec.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 155c6d4..8900f61 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1554,8 +1554,11 @@ static int cn_print_exe_file(struct core_name *cn)
 	int ret;
 
 	exe_file = get_mm_exe_file(current->mm);
-	if (!exe_file)
-		return cn_printf(cn, "(unknown)");
+	if (!exe_file) {
+		char comm[TASK_COMM_LEN];
+		return cn_printf(cn, "%s (path unknown)", get_task_comm(comm,
+					current));
+	}
 
 	pathbuf = kmalloc(PATH_MAX, GFP_TEMPORARY);
 	if (!pathbuf) {
-- 
1.7.4.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH v2 3/3] coredump: escape / in hostname and comm

[Jiri Slaby]

Change every occurence of / in comm and hostname to !. If the process
changes its name to contain /, the core is not dumped (if the
directory tree doesn't exist like that). The same with hostname being
something like myhost/3. Fix this behaviour by using the escape loop
used in %E. (We extract it to a separate function.)

Now both with comm == myprocess/1 and hostname == myhost/1, the core
is dumped like (kernel.core_pattern='core.%p.%e.%h):
core.2349.myprocess!1.myhost!1

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Andi Kleen <andi@firstfloor.org>
---
 fs/exec.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8900f61..dafded4 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1547,10 +1547,17 @@ expand_fail:
 	return ret;
 }
 
+static void cn_escape(char *str)
+{
+	for (; *str; str++)
+		if (*str == '/')
+			*str = '!';
+}
+
 static int cn_print_exe_file(struct core_name *cn)
 {
 	struct file *exe_file;
-	char *pathbuf, *path, *p;
+	char *pathbuf, *path;
 	int ret;
 
 	exe_file = get_mm_exe_file(current->mm);
@@ -1572,9 +1579,7 @@ static int cn_print_exe_file(struct core_name *cn)
 		goto free_buf;
 	}
 
-	for (p = path; *p; p++)
-		if (*p == '/')
-			*p = '!';
+	cn_escape(path);
 
 	ret = cn_printf(cn, "%s", path);
 
@@ -1652,17 +1657,20 @@ static int format_corename(struct core_name *cn, long signr)
 				break;
 			}
 			/* hostname */
-			case 'h':
+			case 'h': {
+				char *namestart = cn->corename + cn->used;
 				down_read(&uts_sem);
 				err = cn_printf(cn, "%s",
 					      utsname()->nodename);
 				up_read(&uts_sem);
+				cn_escape(namestart);
 				break;
+			}
 			/* executable */
 			case 'e': {
 				char comm[TASK_COMM_LEN];
-				err = cn_printf(cn, "%s",
-						get_task_comm(comm, current));
+				cn_escape(get_task_comm(comm, current));
+				err = cn_printf(cn, "%s", comm);
 				break;
 			}
 			case 'E':
-- 
1.7.4.2

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2 1/3] coredump: use get_task_comm for %e filename format

[WANG Cong]

On Thu, 12 May 2011 08:18:11 +0200, Jiri Slaby wrote:

> We currently access current->comm directly. As we have
> prctl(PR_SET_NAME), we need the access be protected by task_lock. This
> is exactly what get_task_comm does, so use it.
> 
> I'm not 100% convinced prctl(PR_SET_NAME) may be called at the time of
> core dump, but the locking won't hurt. Note that siglock is not held in
> format_corename.

John Stultz is working on some patches to convert get_task_common()
to printk %ptc, you will not need to worry about the locking issue.

Thanks.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH v2 1/3] coredump: use get_task_comm for %e filename format

[Oleg Nesterov]

Sorry for delay,

On 05/12, Jiri Slaby wrote:
>
> We currently access current->comm directly. As we have
> prctl(PR_SET_NAME), we need the access be protected by task_lock. This
> is exactly what get_task_comm does, so use it.
>
> I'm not 100% convinced prctl(PR_SET_NAME) may be called at the time of
> core dump,

It can't be called. Apart from current, a sub-thread can change ->comm[]
via /proc/pid/comm, but we already killed all threads.

> but the locking won't hurt.

Agreed, the patch looks correct. but still unneeded.

Oleg.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>