[PATCH -mm] coredump: add support for %d=__get_dumpable() in core name

[PATCH -mm] coredump: add support for %d=__get_dumpable() in core name

[Oleg Nesterov]

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 <vda.linux@googlemail.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/coredump.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/fs/coredump.c b/fs/coredump.c
index 1935b4d..aad8715 100644
--- a/fs/coredump.c
+++ b/fs/coredump.c
@@ -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;
-- 
1.5.5.1

Re: [PATCH -mm] coredump: add support for %d=__get_dumpable() in core name

[Neil Horman]

On Thu, Sep 13, 2012 at 07:28:17PM +0200, Oleg Nesterov wrote:
> 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 <vda.linux@googlemail.com>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> ---
>  fs/coredump.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/coredump.c b/fs/coredump.c
> index 1935b4d..aad8715 100644
> --- a/fs/coredump.c
> +++ b/fs/coredump.c
> @@ -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;
> -- 
> 1.5.5.1
> 
> 
> 
Looks reasonable
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Re: [PATCH -mm] coredump: add support for %d=__get_dumpable() in core name

[Andrew Morton]

On Thu, 13 Sep 2012 19:28:17 +0200
Oleg Nesterov <oleg@redhat.com> wrote:

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

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

> 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?

[PATCH -mm] coredump-add-support-for-%d=__get_dumpable-in-core-name-fix

[Oleg Nesterov]

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

On 09/13, Andrew Morton wrote:
>
> On Thu, 13 Sep 2012 19:28:17 +0200
> Oleg Nesterov <oleg@redhat.com> 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..

> > 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 <oleg@redhat.com>
---
 Documentation/sysctl/kernel.txt |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 6d78841..2907ba6 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -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
-- 
1.5.5.1

[PATCH] coredump: use SUID_DUMPABLE_ENABLED rather than hardcoded 1

[Oleg Nesterov]

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

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 fs/exec.c      |    2 +-
 fs/proc/base.c |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 905e94b..1d3dfc5 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -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);
 
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 21fb230..7f40bb7 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -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;
 }
-- 
1.5.5.1