FAILED: patch "[PATCH] KEYS: trusted: fix writing past end of buffer in" failed to apply to 4.9-stable tree

From a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 Mon Sep 17 00:00:00 2001

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 <[email protected]>.

thanks,

greg k-h

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

From a3c812f7cfd80cf51e8f5b7034f7418f6beb56c1 Mon Sep 17 00:00:00 2001
From: Eric Biggers <[email protected]>
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 <[email protected]>

Cc <[email protected]> # v2.6.38+

Signed-off-by Eric Biggers <[email protected]>

Signed-off-by David Howells <[email protected]>

Reviewed-by Mimi Zohar <[email protected]>

Reviewed-by James Morris <[email protected]>

Signed-off-by James Morris <[email protected]>

security/keys/trusted.c +12 -11

index bd85315cbfeb..98aa89ff7bfd 100644
@@ -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;
 }
 

Hi Greg,

Quoted 5 lines · On Sun, 2017-11-05 at 15:18 +0100, [email protected] 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 <[email protected]>.

Quoted 3 lines

thanks,

greg k-h

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

Quoted 8 lines · Mimi Zohar

Hi Greg,

Quoted 5 lines · On Sun, 2017-11-05 at 15:18 +0100, [email protected] 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 <[email protected]>.

Quoted 6 lines

Quoted 3 lines

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 :(

Quoted 9 lines · Greg KH

Quoted 8 lines · Mimi Zohar

Hi Greg,

Quoted 5 lines · On Sun, 2017-11-05 at 15:18 +0100, [email protected] 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 <[email protected]>.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

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.

(quoted diff) +0 -0

@@ -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?