[Acked] [Trusty][CVE-2015-1805] pipe: iovec: Fix memory corruption when retrying atomic copy as non-atomic

[Andy Whitcroft]

On Mon, Jun 29, 2015 at 02:10:57PM -0700, Kamal Mostafa wrote:
> From: Ben Hutchings <ben at decadent.org.uk>
> 
> pipe_iov_copy_{from,to}_user() may be tried twice with the same iovec,
> the first time atomically and the second time not.  The second attempt
> needs to continue from the iovec position, pipe buffer offset and
> remaining length where the first attempt failed, but currently the
> pipe buffer offset and remaining length are reset.  This will corrupt
> the piped data (possibly also leading to an information leak between
> processes) and may also corrupt kernel memory.
> 
> This was fixed upstream by commits f0d1bec9d58d ("new helper:
> copy_page_from_iter()") and 637b58c2887e ("switch pipe_read() to
> copy_page_to_iter()"), but those aren't suitable for stable.  This fix
> for older kernel versions was made by Seth Jennings for RHEL and I
> have extracted it from their update.
> 
> CVE-2015-1805
> 
> References: https://bugzilla.redhat.com/show_bug.cgi?id=1202855
> Signed-off-by: Ben Hutchings <ben at decadent.org.uk>
> Signed-off-by: Kamal Mostafa <kamal at canonical.com>
> ---
>  fs/pipe.c | 55 ++++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 32 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 0e0752e..3e7ab27 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -117,25 +117,27 @@ void pipe_wait(struct pipe_inode_info *pipe)
>  }
>  
>  static int
> -pipe_iov_copy_from_user(void *to, struct iovec *iov, unsigned long len,
> -			int atomic)
> +pipe_iov_copy_from_user(void *addr, int *offset, struct iovec *iov,
> +			size_t *remaining, int atomic)
>  {
>  	unsigned long copy;
>  
> -	while (len > 0) {
> +	while (*remaining > 0) {
>  		while (!iov->iov_len)
>  			iov++;
> -		copy = min_t(unsigned long, len, iov->iov_len);
> +		copy = min_t(unsigned long, *remaining, iov->iov_len);
>  
>  		if (atomic) {
> -			if (__copy_from_user_inatomic(to, iov->iov_base, copy))
> +			if (__copy_from_user_inatomic(addr + *offset,
> +						      iov->iov_base, copy))
>  				return -EFAULT;
>  		} else {
> -			if (copy_from_user(to, iov->iov_base, copy))
> +			if (copy_from_user(addr + *offset,
> +					   iov->iov_base, copy))
>  				return -EFAULT;
>  		}
> -		to += copy;
> -		len -= copy;
> +		*offset += copy;
> +		*remaining -= copy;
>  		iov->iov_base += copy;
>  		iov->iov_len -= copy;
>  	}
> @@ -143,25 +145,27 @@ pipe_iov_copy_from_user(void *to, struct iovec *iov, unsigned long len,
>  }
>  
>  static int
> -pipe_iov_copy_to_user(struct iovec *iov, const void *from, unsigned long len,
> -		      int atomic)
> +pipe_iov_copy_to_user(struct iovec *iov, void *addr, int *offset,
> +		      size_t *remaining, int atomic)
>  {
>  	unsigned long copy;
>  
> -	while (len > 0) {
> +	while (*remaining > 0) {
>  		while (!iov->iov_len)
>  			iov++;
> -		copy = min_t(unsigned long, len, iov->iov_len);
> +		copy = min_t(unsigned long, *remaining, iov->iov_len);
>  
>  		if (atomic) {
> -			if (__copy_to_user_inatomic(iov->iov_base, from, copy))
> +			if (__copy_to_user_inatomic(iov->iov_base,
> +						    addr + *offset, copy))
>  				return -EFAULT;
>  		} else {
> -			if (copy_to_user(iov->iov_base, from, copy))
> +			if (copy_to_user(iov->iov_base,
> +					 addr + *offset, copy))
>  				return -EFAULT;
>  		}
> -		from += copy;
> -		len -= copy;
> +		*offset += copy;
> +		*remaining -= copy;
>  		iov->iov_base += copy;
>  		iov->iov_len -= copy;
>  	}
> @@ -395,7 +399,7 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
>  			struct pipe_buffer *buf = pipe->bufs + curbuf;
>  			const struct pipe_buf_operations *ops = buf->ops;
>  			void *addr;
> -			size_t chars = buf->len;
> +			size_t chars = buf->len, remaining;
>  			int error, atomic;
>  
>  			if (chars > total_len)
> @@ -409,9 +413,11 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
>  			}
>  
>  			atomic = !iov_fault_in_pages_write(iov, chars);
> +			remaining = chars;
>  redo:
>  			addr = ops->map(pipe, buf, atomic);
> -			error = pipe_iov_copy_to_user(iov, addr + buf->offset, chars, atomic);
> +			error = pipe_iov_copy_to_user(iov, addr, &buf->offset,
> +						      &remaining, atomic);
>  			ops->unmap(pipe, buf, addr);
>  			if (unlikely(error)) {
>  				/*
> @@ -426,7 +432,6 @@ redo:
>  				break;
>  			}
>  			ret += chars;
> -			buf->offset += chars;
>  			buf->len -= chars;
>  
>  			/* Was it a packet buffer? Clean up and exit */
> @@ -531,6 +536,7 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov,
>  		if (ops->can_merge && offset + chars <= PAGE_SIZE) {
>  			int error, atomic = 1;
>  			void *addr;
> +			size_t remaining = chars;
>  
>  			error = ops->confirm(pipe, buf);
>  			if (error)
> @@ -539,8 +545,8 @@ pipe_write(struct kiocb *iocb, const struct iovec *_iov,
>  			iov_fault_in_pages_read(iov, chars);
>  redo1:
>  			addr = ops->map(pipe, buf, atomic);
> -			error = pipe_iov_copy_from_user(offset + addr, iov,
> -							chars, atomic);
> +			error = pipe_iov_copy_from_user(addr, &offset, iov,
> +							&remaining, atomic);
>  			ops->unmap(pipe, buf, addr);
>  			ret = error;
>  			do_wakeup = 1;
> @@ -575,6 +581,8 @@ redo1:
>  			struct page *page = pipe->tmp_page;
>  			char *src;
>  			int error, atomic = 1;
> +			int offset = 0;
> +			size_t remaining;
>  
>  			if (!page) {
>  				page = alloc_page(GFP_HIGHUSER);
> @@ -595,14 +603,15 @@ redo1:
>  				chars = total_len;
>  
>  			iov_fault_in_pages_read(iov, chars);
> +			remaining = chars;
>  redo2:
>  			if (atomic)
>  				src = kmap_atomic(page);
>  			else
>  				src = kmap(page);
>  
> -			error = pipe_iov_copy_from_user(src, iov, chars,
> -							atomic);
> +			error = pipe_iov_copy_from_user(src, &offset, iov,
> +							&remaining, atomic);
>  			if (atomic)
>  				kunmap_atomic(src);
>  			else

Seems to track offset,remaining in a reasonable manner.  As it changes
an interface we should see explosions in build if this is used anywhere
else.

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw