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

[Mimi Zohar <zohar@linux.vnet.ibm.com>]

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;
>  }
> 
>