[natty, natty/ti-omap4 CVE 1/1] TPM: Zero buffer after copying to userspace

[Seth Forshee]

On Mon, Dec 05, 2011 at 04:18:42PM +0000, Andy Whitcroft wrote:
> From: Peter Huewe <huewe.external.infineon at googlemail.com>
> 
> Since the buffer might contain security related data it might be a good idea to
> zero the buffer after we have copied it to userspace.
> 
> This got assigned CVE-2011-1162.
> 
> Signed-off-by: Rajiv Andrade <srajiv at linux.vnet.ibm.com>
> Cc: Stable Kernel <stable at kernel.org>
> Signed-off-by: James Morris <jmorris at namei.org>
> 
> (cherry picked from commit 3321c07ae5068568cd61ac9f4ba749006a7185c9)
> CVE-2011-1162
> BugLink: http://bugs.launchpad.net/bugs/899463
> Signed-off-by: Andy Whitcroft <apw at canonical.com>
> ---
>  drivers/char/tpm/tpm.c |    6 +++++-
>  1 files changed, 5 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 7beb0e2..f59dc23 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -1052,6 +1052,7 @@ ssize_t tpm_read(struct file *file, char __user *buf,
>  {
>  	struct tpm_chip *chip = file->private_data;
>  	ssize_t ret_size;
> +	int rc;
>  
>  	del_singleshot_timer_sync(&chip->user_read_timer);
>  	flush_work_sync(&chip->work);
> @@ -1062,8 +1063,11 @@ ssize_t tpm_read(struct file *file, char __user *buf,
>  			ret_size = size;
>  
>  		mutex_lock(&chip->buffer_mutex);
> -		if (copy_to_user(buf, chip->data_buffer, ret_size))
> +		rc = copy_to_user(buf, chip->data_buffer, ret_size);
> +		memset(chip->data_buffer, 0, ret_size);

I realize this is the same as in the upstream commit. But ...

Just before the context qouted here, tpm_read gets the amount of data in
chip->data_buffer and stores it in ret_size, but then limits ret_size
based off of the size of the user buffer. So potentially there could be
data in the buffer that isn't getting zeroed out. Seems like an
incomplete fix to me.

Unrelated to this patch, I also think tpm_read and tpm_write race on the
data buffer. I think tpm_read ought not set chip->data_pending to 0
until it has finished pulling the data out of the buffer and zeroing it
out.

> +		if (rc)
>  			ret_size = -EFAULT;
> +
>  		mutex_unlock(&chip->buffer_mutex);
>  	}
>  
> -- 
> 1.7.5.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team