LP #345544 - (CVE-2009-0787) ecryptfs stores ram contents in plaintext in the container as padding

[Stefan Bader]

Looks ok to me. ACK

Tim Gardner wrote:
> From 2948c4e71c47eae1de4a259eba1e2af778cbd052 Mon Sep 17 00:00:00 2001
> From: Tyler Hicks <tyhicks at linux.vnet.ibm.com>
> Date: Fri, 20 Mar 2009 01:25:09 -0500
> Subject: [PATCH] eCryptfs: Allocate a variable number of pages for file headers (CVE-2009-0787)
> 
> Bug: #345544
> 
> When allocating the memory used to store the eCryptfs header contents, a
> single, zeroed page was being allocated with get_zeroed_page().
> However, the size of an eCryptfs header is either PAGE_CACHE_SIZE or
> ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE (8192), whichever is larger, and is
> stored in the file's private_data->crypt_stat->num_header_bytes_at_front
> field.
> 
> ecryptfs_write_metadata_to_contents() was using
> num_header_bytes_at_front to decide how many bytes should be written to
> the lower filesystem for the file header.  Unfortunately, at least 8K
> was being written from the page, despite the chance of the single,
> zeroed page being smaller than 8K.  This resulted in random areas of
> kernel memory being written between the 0x1000 and 0x1FFF bytes offsets
> in the eCryptfs file headers if PAGE_SIZE was 4K.
> 
> This patch allocates a variable number of pages, calculated with
> num_header_bytes_at_front, and passes the number of allocated pages
> along to ecryptfs_write_metadata_to_contents().
> 
> Thanks to Florian Streibelt for reporting the data leak and working with
> me to find the problem.  2.6.28 is the only kernel release with this
> vulnerability.  Corresponds to CVE-2009-0787
> 
> Signed-off-by: Tyler Hicks <tyhicks at linux.vnet.ibm.com>
> Acked-by: Dustin Kirkland <kirkland at canonical.com>
> Reviewed-by: Eric Sandeen <sandeen at redhat.com>
> Reviewed-by: Eugene Teo <eugeneteo at kernel.sg>
> Cc: Greg KH <greg at kroah.com>
> Cc: dann frazier <dannf at dannf.org>
> Cc: Serge E. Hallyn <serue at us.ibm.com>
> Cc: Florian Streibelt <florian at f-streibelt.de>
> Cc: stable at kernel.org
> Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>
> (cherry picked from commit 8faece5f906725c10e7a1f6caf84452abadbdc7b)
> 
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> ---
>  fs/ecryptfs/crypto.c |   39 ++++++++++++++++++++++++++-------------
>  1 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index 1607e09..8b65f28 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -1324,14 +1324,13 @@ static int ecryptfs_write_headers_virt(char *page_virt, size_t max,
>  }
>  
>  static int
> -ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat,
> -				    struct dentry *ecryptfs_dentry,
> -				    char *virt)
> +ecryptfs_write_metadata_to_contents(struct dentry *ecryptfs_dentry,
> +				    char *virt, size_t virt_len)
>  {
>  	int rc;
>  
>  	rc = ecryptfs_write_lower(ecryptfs_dentry->d_inode, virt,
> -				  0, crypt_stat->num_header_bytes_at_front);
> +				  0, virt_len);
>  	if (rc)
>  		printk(KERN_ERR "%s: Error attempting to write header "
>  		       "information to lower file; rc = [%d]\n", __func__,
> @@ -1341,7 +1340,6 @@ ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat,
>  
>  static int
>  ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,
> -				 struct ecryptfs_crypt_stat *crypt_stat,
>  				 char *page_virt, size_t size)
>  {
>  	int rc;
> @@ -1351,6 +1349,17 @@ ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,
>  	return rc;
>  }
>  
> +static unsigned long ecryptfs_get_zeroed_pages(gfp_t gfp_mask,
> +					       unsigned int order)
> +{
> +	struct page *page;
> +
> +	page = alloc_pages(gfp_mask | __GFP_ZERO, order);
> +	if (page)
> +		return (unsigned long) page_address(page);
> +	return 0;
> +}
> +
>  /**
>   * ecryptfs_write_metadata
>   * @ecryptfs_dentry: The eCryptfs dentry
> @@ -1367,7 +1376,9 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry)
>  {
>  	struct ecryptfs_crypt_stat *crypt_stat =
>  		&ecryptfs_inode_to_private(ecryptfs_dentry->d_inode)->crypt_stat;
> +	unsigned int order;
>  	char *virt;
> +	size_t virt_len;
>  	size_t size = 0;
>  	int rc = 0;
>  
> @@ -1383,33 +1394,35 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry)
>  		rc = -EINVAL;
>  		goto out;
>  	}
> +	virt_len = crypt_stat->num_header_bytes_at_front;
> +	order = get_order(virt_len);
>  	/* Released in this function */
> -	virt = (char *)get_zeroed_page(GFP_KERNEL);
> +	virt = (char *)ecryptfs_get_zeroed_pages(GFP_KERNEL, order);
>  	if (!virt) {
>  		printk(KERN_ERR "%s: Out of memory\n", __func__);
>  		rc = -ENOMEM;
>  		goto out;
>  	}
> -	rc = ecryptfs_write_headers_virt(virt, PAGE_CACHE_SIZE, &size,
> -					 crypt_stat, ecryptfs_dentry);
> +	rc = ecryptfs_write_headers_virt(virt, virt_len, &size, crypt_stat,
> +					 ecryptfs_dentry);
>  	if (unlikely(rc)) {
>  		printk(KERN_ERR "%s: Error whilst writing headers; rc = [%d]\n",
>  		       __func__, rc);
>  		goto out_free;
>  	}
>  	if (crypt_stat->flags & ECRYPTFS_METADATA_IN_XATTR)
> -		rc = ecryptfs_write_metadata_to_xattr(ecryptfs_dentry,
> -						      crypt_stat, virt, size);
> +		rc = ecryptfs_write_metadata_to_xattr(ecryptfs_dentry, virt,
> +						      size);
>  	else
> -		rc = ecryptfs_write_metadata_to_contents(crypt_stat,
> -							 ecryptfs_dentry, virt);
> +		rc = ecryptfs_write_metadata_to_contents(ecryptfs_dentry, virt,
> +							 virt_len);
>  	if (rc) {
>  		printk(KERN_ERR "%s: Error writing metadata out to lower file; "
>  		       "rc = [%d]\n", __func__, rc);
>  		goto out_free;
>  	}
>  out_free:
> -	free_page((unsigned long)virt);
> +	free_pages((unsigned long)virt, order);
>  out:
>  	return rc;
>  }


-- 

When all other means of communication fail, try words!