[PATCH] ecryptfs: fix memory corruption when storing crypto info in xattrs - Kernel

This is a discussion on [PATCH] ecryptfs: fix memory corruption when storing crypto info in xattrs - Kernel ; 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 ...

+ Reply to Thread
Results 1 to 2 of 2

Thread: [PATCH] ecryptfs: fix memory corruption when storing crypto info in xattrs

  1. [PATCH] ecryptfs: fix memory corruption when storing crypto info in xattrs

    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)

    Signed-off-by: Eric Sandeen
    ---

    Index: linux-2.6.27.x86_64/fs/ecryptfs/crypto.c
    ================================================== =================
    --- linux-2.6.27.x86_64.orig/fs/ecryptfs/crypto.c
    +++ linux-2.6.27.x86_64/fs/ecryptfs/crypto.c
    @@ -1251,6 +1251,7 @@ struct kmem_cache *ecryptfs_header_cache
    /**
    * 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
    @@ -1278,7 +1279,8 @@ struct kmem_cache *ecryptfs_header_cache
    *
    * 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)
    {
    @@ -1296,7 +1298,7 @@ static int ecryptfs_write_headers_virt(c
    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);
    @@ -1368,14 +1370,14 @@ int ecryptfs_write_metadata(struct dentr
    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);
    @@ -1393,8 +1395,7 @@ int ecryptfs_write_metadata(struct dentr
    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;
    }

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] ecryptfs: fix memory corruption when storing crypto info in xattrs

    On Wed, Oct 22, 2008 at 12:14:00PM -0500, Eric Sandeen wrote:
    > 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)
    >
    > Signed-off-by: Eric Sandeen


    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.

    Acked-by: Michael Halcrow

    > ---
    >
    > Index: linux-2.6.27.x86_64/fs/ecryptfs/crypto.c
    > ================================================== =================
    > --- linux-2.6.27.x86_64.orig/fs/ecryptfs/crypto.c
    > +++ linux-2.6.27.x86_64/fs/ecryptfs/crypto.c
    > @@ -1251,6 +1251,7 @@ struct kmem_cache *ecryptfs_header_cache
    > /**
    > * 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
    > @@ -1278,7 +1279,8 @@ struct kmem_cache *ecryptfs_header_cache
    > *
    > * 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)
    > {
    > @@ -1296,7 +1298,7 @@ static int ecryptfs_write_headers_virt(c
    > 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);
    > @@ -1368,14 +1370,14 @@ int ecryptfs_write_metadata(struct dentr
    > 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);
    > @@ -1393,8 +1395,7 @@ int ecryptfs_write_metadata(struct dentr
    > 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;
    > }
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread