Re: [PATCH] supress uid comparison test if core output files arepipes

[Andrew Morton]

On Mon, 22 Feb 2010 15:44:29 -0500 Neil Horman <nhorman@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> 
> 
>  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@xxxxxxx>
AuthorDate: Wed Nov 28 13:59:18 2007 +0100
Commit:     Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxxxxxxxx>
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@xxxxxxx>
    Acked-by: Alan Cox <alan@xxxxxxxxxx>
    Acked-by: Christoph Hellwig <hch@xxxxxx>
    Acked-by: Al Viro <viro@xxxxxxxxxxxxxxxx>
    Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/