coredump: add support for %d=__get_dumpable() in core name

PATCH In review Oleg Nesterov 3 participants 5 messages Sep 13, 2012 – Sep 14, 2012

Subsystems (4)

Documentation VFS exec and binfmt Proc filesystem

lore

Share & tools

lore

Oleg Nesterov

author

2012-09-13 17:26 UTC #0 lore raw

PATCH 1/1 coredump: add support for %d=__get_dumpable() in core name

https://bugzilla.redhat.com/show_bug.cgi?id=787135

Some coredump handlers want to create a core file in a way compatible with standard behavior. Standard behavior with fs.suid_dumpable = 2 is to create core file with uid=gid=0. However, there was no way for coredump handler to know that the process being dumped was suid'ed.

This patch adds the new %d specifier for format_corename() which simply reports __get_dumpable(mm->flags), this is compatible with /proc/sys/fs/suid_dumpable we already have.

By-discussion-with: Denys Vlasenko <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

---
 fs/coredump.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

fs/coredump.c +7 -3

index 1935b4d..aad8715 100644
@@ -149,7 +149,7 @@ put_exe_file:
  * name into corename, which must have space for at least
  * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator.
  */
-static int format_corename(struct core_name *cn, long signr)
+static int format_corename(struct core_name *cn, struct coredump_params *cprm)
 {
 	const struct cred *cred = current_cred();
 	const char *pat_ptr = core_pattern;

@@ -194,9 +194,13 @@ static int format_corename(struct core_name *cn, long signr)
 			case 'g':
 				err = cn_printf(cn, "%d", cred->gid);
 				break;
+			case 'd':
+				err = cn_printf(cn, "%d",
+					__get_dumpable(cprm->mm_flags));
+				break;
 			/* signal that caused the coredump */
 			case 's':
-				err = cn_printf(cn, "%ld", signr);
+				err = cn_printf(cn, "%ld", cprm->signr);
 				break;
 			/* UNIX time of coredump */
 			case 't': {

@@ -524,7 +528,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 	 */
 	clear_thread_flag(TIF_SIGPENDING);
 
-	ispipe = format_corename(&cn, signr);
+	ispipe = format_corename(&cn, &cprm);
 
  	if (ispipe) {
 		int dump_count;

Signature · 1 line

1.5.5.1

Neil Horman

Acked-by on v1 1/1

2012-09-13 18:02 UTC #1 lore raw

Quoted 59 lines · Oleg Nesterov

https://bugzilla.redhat.com/show_bug.cgi?id=787135

This patch adds the new %d specifier for format_corename() which simply reports __get_dumpable(mm->flags), this is compatible with /proc/sys/fs/suid_dumpable we already have.

By-discussion-with: Denys Vlasenko <[email protected]>

Signed-off-by Oleg Nesterov <[email protected]>

---
 fs/coredump.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

fs/coredump.c +7 -3

index 1935b4d..aad8715 100644
@@ -149,7 +149,7 @@ put_exe_file:
  * name into corename, which must have space for at least
  * CORENAME_MAX_SIZE bytes plus one byte for the zero terminator.
  */
-static int format_corename(struct core_name *cn, long signr)
+static int format_corename(struct core_name *cn, struct coredump_params *cprm)
 {
 	const struct cred *cred = current_cred();
 	const char *pat_ptr = core_pattern;

@@ -194,9 +194,13 @@ static int format_corename(struct core_name *cn, long signr)
 			case 'g':
 				err = cn_printf(cn, "%d", cred->gid);
 				break;
+			case 'd':
+				err = cn_printf(cn, "%d",
+					__get_dumpable(cprm->mm_flags));
+				break;
 			/* signal that caused the coredump */
 			case 's':
-				err = cn_printf(cn, "%ld", signr);
+				err = cn_printf(cn, "%ld", cprm->signr);
 				break;
 			/* UNIX time of coredump */
 			case 't': {

@@ -524,7 +528,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
 	 */
 	clear_thread_flag(TIF_SIGPENDING);
 
-	ispipe = format_corename(&cn, signr);
+	ispipe = format_corename(&cn, &cprm);
 
  	if (ispipe) {
 		int dump_count;

Signature · 1 line

1.5.5.1

Looks reasonable

Acked-by Neil Horman <[email protected]>

Andrew Morton

2012-09-13 22:26 UTC #2 lore raw

In reply to Oleg Nesterov

Quoted 1 line · On Thu, 13 Sep 2012 19:28:17 +0200 Oleg Nesterov <[email protected]> wrote:

https://bugzilla.redhat.com/show_bug.cgi?id=787135

Needs a RH bugzilla account, so no Reported-by: for you!

Quoted 8 lines

Some coredump handlers want to create a core file in a way compatible with standard behavior. Standard behavior with fs.suid_dumpable = 2 is to create core file with uid=gid=0. However, there was no way for coredump handler to know that the process being dumped was suid'ed.
This patch adds the new %d specifier for format_corename() which simply reports __get_dumpable(mm->flags), this is compatible with /proc/sys/fs/suid_dumpable we already have.

Please prepare an update to Documentation/sysctl/kernel.txt:core_pattern?

Oleg Nesterov

author

2012-09-14 12:28 UTC #3 lore raw

PATCH 1/1 coredump-add-support-for-%d=__get_dumpable-in-core-name-fix

(fixes coredump-add-support-for-%d=__get_dumpable-in-core-name.patch)

Quoted 7 lines · Andrew Morton

Quoted 1 line · On Thu, 13 Sep 2012 19:28:17 +0200 Oleg Nesterov <[email protected]> wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=787135
Needs a RH bugzilla account, so no Reported-by: for you!

OOPS, sorry, I didn't notice. I guess it is private by mistake..

Quoted 5 lines

Quoted 3 lines
This patch adds the new %d specifier for format_corename() which simply reports __get_dumpable(mm->flags), this is compatible with /proc/sys/fs/suid_dumpable we already have.
Please prepare an update to Documentation/sysctl/kernel.txt:core_pattern?

Yes, thanks, please see below.

Signed-off-by Oleg Nesterov <[email protected]>

---
 Documentation/sysctl/kernel.txt |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Documentation/sysctl/kernel.txt +2 -0

index 6d78841..2907ba6 100644
@@ -181,6 +181,8 @@ core_pattern is used to specify a core dumpfile pattern name.
 	%p	pid
 	%u	uid
 	%g	gid
+	%d	dump mode, matches PR_SET_DUMPABLE and
+		/proc/sys/fs/suid_dumpable
 	%s	signal number
 	%t	UNIX time of dump
 	%h	hostname

Signature · 1 line

1.5.5.1

Oleg Nesterov

author

2012-09-14 12:41 UTC #4 lore raw

PATCH 1/1 coredump: use SUID_DUMPABLE_ENABLED rather than hardcoded 1

In reply to Andrew Morton

Cosmetic. Change setup_new_exec() and task_dumpable() to use SUID_DUMPABLE_ENABLED for /bin/grep.

Signed-off-by Oleg Nesterov <[email protected]>

---
 fs/exec.c      |    2 +-
 fs/proc/base.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

fs/exec.c +1 -1

index 905e94b..1d3dfc5 100644
@@ -1125,7 +1125,7 @@ void setup_new_exec(struct linux_binprm * bprm)
 	current->sas_ss_sp = current->sas_ss_size = 0;
 
 	if (uid_eq(current_euid(), current_uid()) && gid_eq(current_egid(), current_gid()))
-		set_dumpable(current->mm, 1);
+		set_dumpable(current->mm, SUID_DUMPABLE_ENABLED);
 	else
 		set_dumpable(current->mm, suid_dumpable);

fs/proc/base.c +1 -1

index 21fb230..7f40bb7 100644
@@ -1398,7 +1398,7 @@ static int task_dumpable(struct task_struct *task)
 	if (mm)
 		dumpable = get_dumpable(mm);
 	task_unlock(task);
-	if(dumpable == 1)
+	if(dumpable == SUID_DUMPABLE_ENABLED)
 		return 1;
 	return 0;
 }

Signature · 1 line

1.5.5.1