From: Eric Sandeen <sandeen@sandeen.net>
Date: Thu, 23 Oct 2008 12:14:41 -0500
Subject: [fs] ecryptfs: storing crypto info in xattr corrupts mem
Message-id: 4900B101.3000501@sandeen.net
O-Subject: [PATCH RHEL5.3] ecryptfs: fix memory corruption when storing crypto info in xattrs
Bugzilla: 468192
RH-Acked-by: Steven Whitehouse <swhiteho@redhat.com>
RH-Acked-by: Jeff Moyer <jmoyer@redhat.com>

This is for:
Bug 468192 -  writing data to file can fail and cause panic sometimes when using xattr on ecryptfs

The bug only manifests itself when ecryptfs is mounted with the
option to store crypto information in xattrs (rather than as a
file header) and this option is not default, and poorly documented -
so it'd probably rarely be hit.  Still, when mounted with this
option, it's a bad bug.

Below is a patch I sent upstream yesterday, acked by the ecryptfs
maintainer.  jtluka has tested the patch w/ the reproducer.

Thanks,
-Eric

----

When ecryptfs allocates space to write crypto headers into, before
copying it out to file headers or to xattrs, it looks at the value of
crypt_stat->num_header_bytes_at_front to determine how much space it
needs.  This is also used as the file offset to the actual encrypted
data, so for xattr-stored crypto info, the value was zero.

So, we kzalloc'd 0 bytes, and then ran off to write to that memory.
(Which returned as ZERO_SIZE_PTR, so we explode quickly).

The right answer is to always allocate a page to write into; the current
code won't ever write more than that (this is enforced by the
(PAGE_CACHE_SIZE - offset) length in the call to
ecryptfs_generate_key_packet_set).  To be explicit about this, we now
send in a "max" parameter, rather than magically using PAGE_CACHE_SIZE
there.

Also, since the pointer we pass down the callchain eventually gets the
virt_to_page() treatment, we should be using a alloc_page variant, not
kzalloc (see also 7fcba054373d5dfc43d26e243a5c9b92069972ee)

The removal of the memset is okay because it is header information,
which is written to the disk in the clear anyway and is not
sensitive.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Acked-by: Michael Halcrow <mhalcrow@us.ibm.com>

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 3d1df79..1a2621a 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -1190,6 +1190,7 @@ struct kmem_cache *ecryptfs_header_cache_2;
 /**
  * ecryptfs_write_headers_virt
  * @page_virt: The virtual address to write the headers to
+ * @max: The size of memory allocated at page_virt
  * @size: Set to the number of bytes written by this function
  * @crypt_stat: The cryptographic context
  * @ecryptfs_dentry: The eCryptfs dentry
@@ -1217,7 +1218,8 @@ struct kmem_cache *ecryptfs_header_cache_2;
  *
  * Returns zero on success
  */
-static int ecryptfs_write_headers_virt(char *page_virt, size_t *size,
+static int ecryptfs_write_headers_virt(char *page_virt, size_t max,
+				       size_t *size,
 				       struct ecryptfs_crypt_stat *crypt_stat,
 				       struct dentry *ecryptfs_dentry)
 {
@@ -1235,7 +1237,7 @@ static int ecryptfs_write_headers_virt(char *page_virt, size_t *size,
 	offset += written;
 	rc = ecryptfs_generate_key_packet_set((page_virt + offset), crypt_stat,
 					      ecryptfs_dentry, &written,
-					      PAGE_CACHE_SIZE - offset);
+					      max - offset);
 	if (rc)
 		ecryptfs_printk(KERN_WARNING, "Error generating key packet "
 				"set; rc = [%d]\n", rc);
@@ -1307,14 +1309,14 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry)
 		goto out;
 	}
 	/* Released in this function */
-	virt = kzalloc(crypt_stat->num_header_bytes_at_front, GFP_KERNEL);
+	virt = (char *)get_zeroed_page(GFP_KERNEL);
 	if (!virt) {
 		printk(KERN_ERR "%s: Out of memory\n", __func__);
 		rc = -ENOMEM;
 		goto out;
 	}
-	rc = ecryptfs_write_headers_virt(virt, &size, crypt_stat,
-					 ecryptfs_dentry);
+	rc = ecryptfs_write_headers_virt(virt, PAGE_CACHE_SIZE, &size,
+					 crypt_stat, ecryptfs_dentry);
 	if (unlikely(rc)) {
 		printk(KERN_ERR "%s: Error whilst writing headers; rc = [%d]\n",
 		       __func__, rc);
@@ -1332,8 +1334,7 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry)
 		goto out_free;
 	}
 out_free:
-	memset(virt, 0, crypt_stat->num_header_bytes_at_front);
-	kfree(virt);
+	free_page((unsigned long)virt);
 out:
 	return rc;
 }