diff mbox

[Precise,Trusty,SRU,1/1,stable-only] pipe: Fix buffer offset after partially failed read

Message ID 1459350539-18569-1-git-send-email-kamal@canonical.com
State New
Headers show

Commit Message

Kamal Mostafa March 30, 2016, 3:08 p.m. UTC
From: Ben Hutchings <ben@decadent.org.uk>

BugLink: https://launchpad.net/bugs/1563916

This 'stable-only' patch (lifted from 3.2.78) is required to fix CVE-2016-0774
in Precise and Trusty.

The patch is pending in 3.13-stable, but Trusty needs it asap.

Precise can acquire it by this SRU or via 3.2.78.

 -Kamal

-----8<-----------------------------------------------------------------

BugLink: https://launchpad.net/bugs/1563916

Quoting the RHEL advisory:

> It was found that the fix for CVE-2015-1805 incorrectly kept buffer
> offset and buffer length in sync on a failed atomic read, potentially
> resulting in a pipe buffer state corruption. A local, unprivileged user
> could use this flaw to crash the system or leak kernel memory to user
> space. (CVE-2016-0774, Moderate)

The same flawed fix was applied to stable branches from 2.6.32.y to
3.14.y inclusive, and I was able to reproduce the issue on 3.2.y.
We need to give pipe_iov_copy_to_user() a separate offset variable
and only update the buffer offset if it succeeds.

References: https://rhn.redhat.com/errata/RHSA-2016-0103.html
Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
References: https://lkml.org/lkml/2016/2/23/812
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
---
 fs/pipe.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Luis Henriques March 31, 2016, 1:21 p.m. UTC | #1
On Wed, Mar 30, 2016 at 08:08:59AM -0700, Kamal Mostafa wrote:
> From: Ben Hutchings <ben@decadent.org.uk>
> 
> BugLink: https://launchpad.net/bugs/1563916
> 
> This 'stable-only' patch (lifted from 3.2.78) is required to fix CVE-2016-0774
> in Precise and Trusty.
> 
> The patch is pending in 3.13-stable, but Trusty needs it asap.
> 
> Precise can acquire it by this SRU or via 3.2.78.
>

This fix has been already applied to upstream stable kernels 2.6.32, 3.2,
3.12 and will soon be in 3.13.  The Precise fix is already applied to the
master-next branch through upstream stable release 3.2.78.

Cheers,
--
Luís


>  -Kamal
> 
> -----8<-----------------------------------------------------------------
> 
> BugLink: https://launchpad.net/bugs/1563916
> 
> Quoting the RHEL advisory:
> 
> > It was found that the fix for CVE-2015-1805 incorrectly kept buffer
> > offset and buffer length in sync on a failed atomic read, potentially
> > resulting in a pipe buffer state corruption. A local, unprivileged user
> > could use this flaw to crash the system or leak kernel memory to user
> > space. (CVE-2016-0774, Moderate)
> 
> The same flawed fix was applied to stable branches from 2.6.32.y to
> 3.14.y inclusive, and I was able to reproduce the issue on 3.2.y.
> We need to give pipe_iov_copy_to_user() a separate offset variable
> and only update the buffer offset if it succeeds.
> 
> References: https://rhn.redhat.com/errata/RHSA-2016-0103.html
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> References: https://lkml.org/lkml/2016/2/23/812
> Signed-off-by: Kamal Mostafa <kamal@canonical.com>
> ---
>  fs/pipe.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/pipe.c b/fs/pipe.c
> index 47f79ac..c281867 100644
> --- a/fs/pipe.c
> +++ b/fs/pipe.c
> @@ -407,6 +407,7 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
>  			void *addr;
>  			size_t chars = buf->len, remaining;
>  			int error, atomic;
> +			int offset;
>  
>  			if (chars > total_len)
>  				chars = total_len;
> @@ -420,9 +421,10 @@ pipe_read(struct kiocb *iocb, const struct iovec *_iov,
>  
>  			atomic = !iov_fault_in_pages_write(iov, chars);
>  			remaining = chars;
> +			offset = buf->offset;
>  redo:
>  			addr = ops->map(pipe, buf, atomic);
> -			error = pipe_iov_copy_to_user(iov, addr, &buf->offset,
> +			error = pipe_iov_copy_to_user(iov, addr, &offset,
>  						      &remaining, atomic);
>  			ops->unmap(pipe, buf, addr);
>  			if (unlikely(error)) {
> @@ -438,6 +440,7 @@ redo:
>  				break;
>  			}
>  			ret += chars;
> +			buf->offset += chars;
>  			buf->len -= chars;
>  
>  			/* Was it a packet buffer? Clean up and exit */
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
Tim Gardner March 31, 2016, 1:28 p.m. UTC | #2

Kamal Mostafa March 31, 2016, 4:13 p.m. UTC | #3
Applied to Trusty.  Precise picked it up via v3.2.78.
 
 -Kamal
diff mbox

Patch

diff --git a/fs/pipe.c b/fs/pipe.c
index 47f79ac..c281867 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -407,6 +407,7 @@  pipe_read(struct kiocb *iocb, const struct iovec *_iov,
 			void *addr;
 			size_t chars = buf->len, remaining;
 			int error, atomic;
+			int offset;
 
 			if (chars > total_len)
 				chars = total_len;
@@ -420,9 +421,10 @@  pipe_read(struct kiocb *iocb, const struct iovec *_iov,
 
 			atomic = !iov_fault_in_pages_write(iov, chars);
 			remaining = chars;
+			offset = buf->offset;
 redo:
 			addr = ops->map(pipe, buf, atomic);
-			error = pipe_iov_copy_to_user(iov, addr, &buf->offset,
+			error = pipe_iov_copy_to_user(iov, addr, &offset,
 						      &remaining, atomic);
 			ops->unmap(pipe, buf, addr);
 			if (unlikely(error)) {
@@ -438,6 +440,7 @@  redo:
 				break;
 			}
 			ret += chars;
+			buf->offset += chars;
 			buf->len -= chars;
 
 			/* Was it a packet buffer? Clean up and exit */