Skip to content

Commit

Permalink
ecryptfs: fix memory corruption when storing crypto info in xattrs
Browse files Browse the repository at this point in the history
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 7fcba05)

Signed-off-by: Eric Sandeen <[email protected]>
Acked-by: Michael Halcrow <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Eric Sandeen authored and torvalds committed Oct 30, 2008
1 parent ce05fcc commit 87b811c
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions fs/ecryptfs/crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -1251,6 +1251,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
Expand Down Expand Up @@ -1278,7 +1279,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)
{
Expand All @@ -1296,7 +1298,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);
Expand Down Expand Up @@ -1368,14 +1370,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);
Expand All @@ -1393,8 +1395,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;
}
Expand Down

0 comments on commit 87b811c

Please sign in to comment.