linux-kernel - Re: [PATCH] supress uid comparison test if core output files are pipes

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20100224135053.e75800c1.akpm@linux-foundation.org>
Date:	Wed, 24 Feb 2010 13:50:53 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Neil Horman <nhorman@...driver.com>
Cc:	oleg@...hat.com, viro@...iv.linux.org.uk,
	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH] supress uid comparison test if core output files are
 pipes

On Mon, 22 Feb 2010 15:44:29 -0500 Neil Horman <nhorman@...driver.com> wrote:

> Modify uid check in do_coredump so as to not apply it in the case of pipes
> 
> So this just got noticed in testing.  The end of do_coredump validates the uid
> of the inode for the created file against the uid of the crashing process to
> ensure that no one can pre-create a core file with different ownership and grab
> the information contained in the core when they shouldn' tbe able to.  This
> causes failures when using pipes for a core dumps if the crashing process is not
> root, which is the uid of the pipe when it is created.
> 
> The fix is simple.  Since the check for matching uid's isn't relevant for pipes
> (a process can't create a pipe that the uermodehelper code will open anyway), we
> can just just skip it in the event ispipe is non-zero
> 
> Signed-off-by: Neil Horman <nhorman@...driver.com>
> 
> 
>  exec.c |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/exec.c b/fs/exec.c
> index 6303d18..6af2214 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1987,8 +1987,9 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
>  	/*
>  	 * Dont allow local users get cute and trick others to coredump
>  	 * into their pre-created files:
> +	 * Note, this is not relevant for pipes
>  	 */
> -	if (inode->i_uid != current_fsuid())
> +	if (!ispipe && (inode->i_uid != current_fsuid()))
>  		goto close_fail;
>  	if (!cprm.file->f_op)
>  		goto close_fail;

hm, this actually appears to fix a regression, added by:



commit c46f739dd39db3b07ab5deb4e3ec81e1c04a91af
Author:     Ingo Molnar <mingo@...e.hu>
AuthorDate: Wed Nov 28 13:59:18 2007 +0100
Commit:     Linus Torvalds <torvalds@...dy.linux-foundation.org>
CommitDate: Wed Nov 28 10:58:01 2007 -0800

    vfs: coredumping fix
    
    fix: http://bugzilla.kernel.org/show_bug.cgi?id=3043
    
    only allow coredumping to the same uid that the coredumping
    task runs under.
    
    Signed-off-by: Ingo Molnar <mingo@...e.hu>
    Acked-by: Alan Cox <alan@...hat.com>
    Acked-by: Christoph Hellwig <hch@....de>
    Acked-by: Al Viro <viro@....linux.org.uk>
    Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>

diff --git a/fs/exec.c b/fs/exec.c
index 4ccaaa4..282240a 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1780,6 +1780,12 @@ int do_coredump(long signr, int exit_code, struct pt_regs * regs)
 	   but keep the previous behaviour for now. */
 	if (!ispipe && !S_ISREG(inode->i_mode))
 		goto close_fail;
+	/*
+	 * Dont allow local users get cute and trick others to coredump
+	 * into their pre-created files:
+	 */
+	if (inode->i_uid != current->fsuid)
+		goto close_fail;
 	if (!file->f_op)
 		goto close_fail;
 	if (!file->f_op->write)

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