All of lore.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree
@ 2017-11-05 14:18 gregkh
  2017-11-06 11:06 ` Mimi Zohar
  0 siblings, 1 reply; 5+ messages in thread
From: gregkh @ 2017-11-05 14:18 UTC (permalink / raw)
  To: ebiggers, ben, dhowells, james.l.morris, stable, zohar; +Cc: stable


The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

>From a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Thu, 2 Nov 2017 00:47:12 +0000
Subject: [PATCH] KEYS: trusted: fix writing past end of buffer in
 trusted_read()

When calling keyctl_read() on a key of type "trusted", if the
user-supplied buffer was too small, the kernel ignored the buffer length
and just wrote past the end of the buffer, potentially corrupting
userspace memory.  Fix it by instead returning the size required, as per
the documentation for keyctl_read().

We also don't even fill the buffer at all in this case, as this is
slightly easier to implement than doing a short read, and either
behavior appears to be permitted.  It also makes it match the behavior
of the "encrypted" key type.

Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
Reported-by: Ben Hutchings <ben@decadent.org.uk>
Cc: <stable@vger.kernel.org> # v2.6.38+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Reviewed-by: James Morris <james.l.morris@oracle.com>
Signed-off-by: James Morris <james.l.morris@oracle.com>

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index bd85315cbfeb..98aa89ff7bfd 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1147,20 +1147,21 @@ static long trusted_read(const struct key *key, char __user *buffer,
 	p = dereference_key_locked(key);
 	if (!p)
 		return -EINVAL;
-	if (!buffer || buflen <= 0)
-		return 2 * p->blob_len;
-	ascii_buf = kmalloc(2 * p->blob_len, GFP_KERNEL);
-	if (!ascii_buf)
-		return -ENOMEM;
 
-	bufp = ascii_buf;
-	for (i = 0; i < p->blob_len; i++)
-		bufp = hex_byte_pack(bufp, p->blob[i]);
-	if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) {
+	if (buffer && buflen >= 2 * p->blob_len) {
+		ascii_buf = kmalloc(2 * p->blob_len, GFP_KERNEL);
+		if (!ascii_buf)
+			return -ENOMEM;
+
+		bufp = ascii_buf;
+		for (i = 0; i < p->blob_len; i++)
+			bufp = hex_byte_pack(bufp, p->blob[i]);
+		if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
+			kzfree(ascii_buf);
+			return -EFAULT;
+		}
 		kzfree(ascii_buf);
-		return -EFAULT;
 	}
-	kzfree(ascii_buf);
 	return 2 * p->blob_len;
 }
 

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree
  2017-11-05 14:18 FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree gregkh
@ 2017-11-06 11:06 ` Mimi Zohar
  2017-11-10 11:49   ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2017-11-06 11:06 UTC (permalink / raw)
  To: gregkh, ebiggers, ben, dhowells, james.l.morris, stable

Hi Greg,

On Sun, 2017-11-05 at 15:18 +0100, gregkh@linuxfoundation.org wrote:
> The patch below does not apply to the 4.9-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> thanks,
> 
> greg k-h

This commit needs to prereq commit ee618b4619b7 "KEYS: trusted:
sanitize all key material".

Mimi

> ------------------ original commit in Linus's tree ------------------
> 
> From a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 Mon Sep 17 00:00:00 2001
> From: Eric Biggers <ebiggers@google.com>
> Date: Thu, 2 Nov 2017 00:47:12 +0000
> Subject: [PATCH] KEYS: trusted: fix writing past end of buffer in
>  trusted_read()
> 
> When calling keyctl_read() on a key of type "trusted", if the
> user-supplied buffer was too small, the kernel ignored the buffer length
> and just wrote past the end of the buffer, potentially corrupting
> userspace memory.  Fix it by instead returning the size required, as per
> the documentation for keyctl_read().
> 
> We also don't even fill the buffer at all in this case, as this is
> slightly easier to implement than doing a short read, and either
> behavior appears to be permitted.  It also makes it match the behavior
> of the "encrypted" key type.
> 
> Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
> Reported-by: Ben Hutchings <ben@decadent.org.uk>
> Cc: <stable@vger.kernel.org> # v2.6.38+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Reviewed-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Reviewed-by: James Morris <james.l.morris@oracle.com>
> Signed-off-by: James Morris <james.l.morris@oracle.com>
> 
> diff --git a/security/keys/trusted.c b/security/keys/trusted.c
> index bd85315cbfeb..98aa89ff7bfd 100644
> --- a/security/keys/trusted.c
> +++ b/security/keys/trusted.c
> @@ -1147,20 +1147,21 @@ static long trusted_read(const struct key *key, char __user *buffer,
>  	p = dereference_key_locked(key);
>  	if (!p)
>  		return -EINVAL;
> -	if (!buffer || buflen <= 0)
> -		return 2 * p->blob_len;
> -	ascii_buf = kmalloc(2 * p->blob_len, GFP_KERNEL);
> -	if (!ascii_buf)
> -		return -ENOMEM;
> 
> -	bufp = ascii_buf;
> -	for (i = 0; i < p->blob_len; i++)
> -		bufp = hex_byte_pack(bufp, p->blob[i]);
> -	if ((copy_to_user(buffer, ascii_buf, 2 * p->blob_len)) != 0) {
> +	if (buffer && buflen >= 2 * p->blob_len) {
> +		ascii_buf = kmalloc(2 * p->blob_len, GFP_KERNEL);
> +		if (!ascii_buf)
> +			return -ENOMEM;
> +
> +		bufp = ascii_buf;
> +		for (i = 0; i < p->blob_len; i++)
> +			bufp = hex_byte_pack(bufp, p->blob[i]);
> +		if (copy_to_user(buffer, ascii_buf, 2 * p->blob_len) != 0) {
> +			kzfree(ascii_buf);
> +			return -EFAULT;
> +		}
>  		kzfree(ascii_buf);
> -		return -EFAULT;
>  	}
> -	kzfree(ascii_buf);
>  	return 2 * p->blob_len;
>  }
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree
  2017-11-06 11:06 ` Mimi Zohar
@ 2017-11-10 11:49   ` Greg KH
  2017-11-10 15:56     ` Mimi Zohar
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2017-11-10 11:49 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: ebiggers, ben, dhowells, james.l.morris, stable

On Mon, Nov 06, 2017 at 06:06:19AM -0500, Mimi Zohar wrote:
> Hi Greg,
> 
> On Sun, 2017-11-05 at 15:18 +0100, gregkh@linuxfoundation.org wrote:
> > The patch below does not apply to the 4.9-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> > 
> > thanks,
> > 
> > greg k-h
> 
> This commit needs to prereq commit ee618b4619b7 "KEYS: trusted:
> sanitize all key material".

Thanks, that fixes the issue for 4.4 and 4.9, but not for 3.18 :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree
  2017-11-10 11:49   ` Greg KH
@ 2017-11-10 15:56     ` Mimi Zohar
  2017-11-10 19:21       ` Eric Biggers
  0 siblings, 1 reply; 5+ messages in thread
From: Mimi Zohar @ 2017-11-10 15:56 UTC (permalink / raw)
  To: Greg KH; +Cc: ebiggers, ben, dhowells, james.l.morris, stable

On Fri, 2017-11-10 at 12:49 +0100, Greg KH wrote:
> On Mon, Nov 06, 2017 at 06:06:19AM -0500, Mimi Zohar wrote:
> > Hi Greg,
> > 
> > On Sun, 2017-11-05 at 15:18 +0100, gregkh@linuxfoundation.org wrote:
> > > The patch below does not apply to the 4.9-stable tree.
> > > If someone wants it applied there, or to any other stable or longterm
> > > tree, then please email the backport, including the original git commit
> > > id to <stable@vger.kernel.org>.
> > > 
> > > thanks,
> > > 
> > > greg k-h
> > 
> > This commit needs to prereq commit ee618b4619b7 "KEYS: trusted:
> > sanitize all key material".
> 
> Thanks, that fixes the issue for 4.4 and 4.9, but not for 3.18 :(

Commit 146aa8b "KEYS: Merge the type-specific data with the payload
data" introduced the change trusted_destroy(), but it is a rather big
patch.

@@ -1114,12 +1114,12 @@ static long trusted_read(const struct key
*key, char __user 
*buffer,
  */
 static void trusted_destroy(struct key *key)
 {
-       struct trusted_key_payload *p = key->payload.data;
+       struct trusted_key_payload *p = key->payload.data[0];
 
        if (!p)
                return;
        memset(p->key, 0, p->key_len);
-       kfree(key->payload.data);
+       kfree(key->payload.data[0]);
 }
 
Perhaps David has back ported this patch already. David?  Otherwise,
Eric could you create a patch that applies directly to the stable tree
linux-3.18.y?

thanks,

Mimi

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree
  2017-11-10 15:56     ` Mimi Zohar
@ 2017-11-10 19:21       ` Eric Biggers
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2017-11-10 19:21 UTC (permalink / raw)
  To: Mimi Zohar; +Cc: Greg KH, ben, dhowells, james.l.morris, stable

On Fri, Nov 10, 2017 at 10:56:46AM -0500, Mimi Zohar wrote:
> On Fri, 2017-11-10 at 12:49 +0100, Greg KH wrote:
> > On Mon, Nov 06, 2017 at 06:06:19AM -0500, Mimi Zohar wrote:
> > > Hi Greg,
> > > 
> > > On Sun, 2017-11-05 at 15:18 +0100, gregkh@linuxfoundation.org wrote:
> > > > The patch below does not apply to the 4.9-stable tree.
> > > > If someone wants it applied there, or to any other stable or longterm
> > > > tree, then please email the backport, including the original git commit
> > > > id to <stable@vger.kernel.org>.
> > > > 
> > > > thanks,
> > > > 
> > > > greg k-h
> > > 
> > > This commit needs to prereq commit ee618b4619b7 "KEYS: trusted:
> > > sanitize all key material".
> > 
> > Thanks, that fixes the issue for 4.4 and 4.9, but not for 3.18 :(
> 
> Commit 146aa8b "KEYS: Merge the type-specific data with the payload
> data" introduced the change trusted_destroy(), but it is a rather big
> patch.
> 
> @@ -1114,12 +1114,12 @@ static long trusted_read(const struct key
> *key, char __user�
> *buffer,
> � */
> �static void trusted_destroy(struct key *key)
> �{
> -�������struct trusted_key_payload *p = key->payload.data;
> +�������struct trusted_key_payload *p = key->payload.data[0];
> �
> ��������if (!p)
> ����������������return;
> ��������memset(p->key, 0, p->key_len);
> -�������kfree(key->payload.data);
> +�������kfree(key->payload.data[0]);
> �}
> �
> Perhaps David has back ported this patch already. David? �Otherwise,
> Eric could you create a patch that applies directly to the stable tree
> linux-3.18.y?
> 

I'll send backports of "KEYS: trusted: sanitize all key material" and "KEYS:
trusted: fix writing past end of buffer in trusted_read()".  We don't need
"KEYS: Merge the type-specific data with the payload data", as far as I know; it
seems to be cleanup/refactoring only.

Eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-10 19:21 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-05 14:18 FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree gregkh
2017-11-06 11:06 ` Mimi Zohar
2017-11-10 11:49   ` Greg KH
2017-11-10 15:56     ` Mimi Zohar
2017-11-10 19:21       ` Eric Biggers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.