eCryptfs: extent offset fix, crypto performance, and larger header regions
cover eCryptfs: extent offset fix, crypto performance, and larger header regions
This patch set includes some minor fixes for eCryptfs, including:
- An enhancement to allow larger header regions
- A bugfix for hosts with page size >4K
- An enhancement to reduce crypto overhead
I recommend merging at least the bugfix into 2.6.24.
PATCH 1/3 eCryptfs: Track header bytes rather than extents
Remove internal references to header extents; just keep track of header bytes instead. Headers can easily span multiple pages with the recent persistent file changes.
Signed-off-by Michael Halcrow <[email protected]>
--- fs/ecryptfs/crypto.c | 98 +++++++++++++++-------------------------- fs/ecryptfs/ecryptfs_kernel.h | 3 +- fs/ecryptfs/inode.c | 8 +-- fs/ecryptfs/main.c | 5 -- fs/ecryptfs/mmap.c | 21 +++++---- 5 files changed, 51 insertions(+), 84 deletions(-)
fs/ecryptfs/crypto.c +35 -63
@@ -361,8 +361,7 @@ out:void ecryptfs_lower_offset_for_extent (loff_t *offset,loff_t extent_num,struct ecryptfs_crypt_stat *crypt_stat) { - (*offset) = ((crypt_stat->extent_size - * crypt_stat->num_header_extents_at_front) + (*offset) = (crypt_stat->num_header_bytes_at_front + (crypt_stat->extent_size * extent_num)); }
@@ -826,15 +825,13 @@ void ecryptfs_set_default_sizes(struct ecryptfs_crypt_stat *crypt_stat)set_extent_mask_and_shift (crypt_stat); crypt_stat->iv_bytes =ECRYPTFS_DEFAULT_IV_BYTES ;if (crypt_stat->flags &ECRYPTFS_METADATA_IN_XATTR ) - crypt_stat->num_header_extents_at_front =0 ; + crypt_stat->num_header_bytes_at_front =0 ;else {if (PAGE_CACHE_SIZE <=ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE ) - crypt_stat->num_header_extents_at_front = - (ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE - / crypt_stat->extent_size); + crypt_stat->num_header_bytes_at_front = +ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE ;else - crypt_stat->num_header_extents_at_front = - (PAGE_CACHE_SIZE / crypt_stat->extent_size); + crypt_stat->num_header_bytes_at_front =PAGE_CACHE_SIZE ; } }
@@ -1220,7 +1217,8 @@ ecryptfs_write_header_metadata(char *virt, header_extent_size = (u32 )crypt_stat->extent_size; num_header_extents_at_front = - (u16 )crypt_stat->num_header_extents_at_front; + (u16)(crypt_stat->num_header_bytes_at_front + / crypt_stat->extent_size); header_extent_size =cpu_to_be32 (header_extent_size);memcpy (virt, &header_extent_size,4 ); virt +=4 ;
@@ -1295,40 +1293,16 @@ static int ecryptfs_write_headers_virt(char *page_virt, size_t *size,static int ecryptfs_write_metadata_to_contents (struct ecryptfs_crypt_stat *crypt_stat,struct dentry *ecryptfs_dentry, -char *page_virt) +char *virt) { -int current_header_page; -int header_pages;int rc; - rc =ecryptfs_write_lower (ecryptfs_dentry->d_inode, page_virt, -0 ,PAGE_CACHE_SIZE ); -if (rc) { + rc =ecryptfs_write_lower (ecryptfs_dentry->d_inode, virt, +0 , crypt_stat->num_header_bytes_at_front); +if (rc)printk (KERN_ERR "%s: Error attempting to write header " "information to lower file; rc = [%d]\n" , __FUNCTION__, rc); - goto out; - } - header_pages = ((crypt_stat->extent_size - * crypt_stat->num_header_extents_at_front) - /PAGE_CACHE_SIZE ); -memset (page_virt,0 ,PAGE_CACHE_SIZE ); - current_header_page =1 ; -while (current_header_page < header_pages) { -loff_t offset; - - offset = (((loff_t )current_header_page) <<PAGE_CACHE_SHIFT ); -if ((rc =ecryptfs_write_lower (ecryptfs_dentry->d_inode, - page_virt, offset, -PAGE_CACHE_SIZE ))) { -printk (KERN_ERR "%s: Error attempting to write header " -"information to lower file; rc = [%d]\n" , - __FUNCTION__, rc); - goto out; - } - current_header_page++; - } -out:return rc; }
@@ -1354,15 +1328,13 @@ ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry,* retrieved via a prompt. Exactly what happens at this point should * be policy-dependent. * -* TODO: Support header information spanning multiple pages -* * Returns zero on success; non-zero on error */ int ecryptfs_write_metadata (struct dentry *ecryptfs_dentry) {struct ecryptfs_crypt_stat *crypt_stat = &ecryptfs_inode_to_private (ecryptfs_dentry->d_inode)->crypt_stat; -char *page_virt; +char *virt;size_t size =0 ;int rc =0 ;
@@ -1373,40 +1345,39 @@ int ecryptfs_write_metadata(struct dentry *ecryptfs_dentry) goto out; } }else { +printk (KERN_WARNING "%s: Encrypted flag not set\n" , + __FUNCTION__); rc = -EINVAL ; -ecryptfs_printk (KERN_WARNING , -"Called with crypt_stat->encrypted == 0\n" ); goto out; }/* Released in this function */ - page_virt =kmem_cache_zalloc (ecryptfs_header_cache_0,GFP_USER ); -if (!page_virt) { -ecryptfs_printk (KERN_ERR ,"Out of memory\n" ); + virt =kzalloc (crypt_stat->num_header_bytes_at_front,GFP_KERNEL ); +if (!virt) { +printk (KERN_ERR "%s: Out of memory\n" , __FUNCTION__); rc = -ENOMEM ; goto out; } - rc =ecryptfs_write_headers_virt (page_virt, &size, crypt_stat, - ecryptfs_dentry); + rc =ecryptfs_write_headers_virt (virt, &size, crypt_stat, + ecryptfs_dentry);if (unlikely (rc)) { -ecryptfs_printk (KERN_ERR ,"Error whilst writing headers\n" ); -memset (page_virt,0 ,PAGE_CACHE_SIZE ); +printk (KERN_ERR "%s: Error whilst writing headers; rc = [%d]\n" , + __FUNCTION__, rc); goto out_free; }if (crypt_stat->flags &ECRYPTFS_METADATA_IN_XATTR ) rc =ecryptfs_write_metadata_to_xattr (ecryptfs_dentry, - crypt_stat, page_virt, - size); + crypt_stat, virt, size);else rc =ecryptfs_write_metadata_to_contents (crypt_stat, - ecryptfs_dentry, - page_virt); + ecryptfs_dentry, virt);if (rc) { -printk (KERN_ERR "Error writing metadata out to lower file; " -"rc = [%d]\n" , rc); +printk (KERN_ERR "%s: Error writing metadata out to lower file; " +"rc = [%d]\n" , __FUNCTION__, rc); goto out_free; } out_free: -kmem_cache_free (ecryptfs_header_cache_0, page_virt); +memset (virt,0 , crypt_stat->num_header_bytes_at_front); +kfree (virt); out:return rc; }
@@ -1426,16 +1397,16 @@ static int parse_header_metadata(struct ecryptfs_crypt_stat *crypt_stat, virt +=sizeof (u32);memcpy (&num_header_extents_at_front, virt,sizeof (u16)); num_header_extents_at_front =be16_to_cpu (num_header_extents_at_front); - crypt_stat->num_header_extents_at_front = - (int )num_header_extents_at_front; + crypt_stat->num_header_bytes_at_front = + (((size_t )num_header_extents_at_front + * (size_t )header_extent_size)); (*bytes_read) = (sizeof (u32) +sizeof (u16));if ((validate_header_size ==ECRYPTFS_VALIDATE_HEADER_SIZE ) - && ((crypt_stat->extent_size - * crypt_stat->num_header_extents_at_front) + && (crypt_stat->num_header_bytes_at_front <ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE )) { rc = -EINVAL ; -printk (KERN_WARNING "Invalid number of header extents: [%zd]\n" , - crypt_stat->num_header_extents_at_front); +printk (KERN_WARNING "Invalid header size: [%zd]\n" , + crypt_stat->num_header_bytes_at_front); }return rc; }
@@ -1450,7 +1421,8 @@ static int parse_header_metadata(struct ecryptfs_crypt_stat *crypt_stat,*/ static void set_default_header_data (struct ecryptfs_crypt_stat *crypt_stat) { - crypt_stat->num_header_extents_at_front =2 ; + crypt_stat->num_header_bytes_at_front = +ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE ; }/**
fs/ecryptfs/ecryptfs_kernel.h +1 -2
@@ -237,7 +237,7 @@ struct ecryptfs_crypt_stat {u32 flags;unsigned int file_version;size_t iv_bytes; -size_t num_header_extents_at_front; +size_t num_header_bytes_at_front;size_t extent_size;/* Data extent size; default is 4096 */ size_t key_size;size_t extent_shift;
@@ -521,7 +521,6 @@ extern struct kmem_cache *ecryptfs_file_info_cache;extern struct kmem_cache *ecryptfs_dentry_info_cache;extern struct kmem_cache *ecryptfs_inode_info_cache;extern struct kmem_cache *ecryptfs_sb_info_cache; -extern struct kmem_cache *ecryptfs_header_cache_0;extern struct kmem_cache *ecryptfs_header_cache_1;extern struct kmem_cache *ecryptfs_header_cache_2;extern struct kmem_cache *ecryptfs_xattr_cache;
fs/ecryptfs/inode.c +3 -5
@@ -378,8 +378,7 @@ static struct dentry *ecryptfs_lookup(struct inode *dir, struct dentry *dentry, dentry->d_sb)->mount_crypt_stat;if (mount_crypt_stat->flags &ECRYPTFS_ENCRYPTED_VIEW_ENABLED ) {if (crypt_stat->flags &ECRYPTFS_METADATA_IN_XATTR ) - file_size = ((crypt_stat->extent_size - * crypt_stat->num_header_extents_at_front) + file_size = (crypt_stat->num_header_bytes_at_front +i_size_read (lower_dentry->d_inode));else file_size =i_size_read (lower_dentry->d_inode);
@@ -697,7 +696,7 @@ ecryptfs_put_link(struct dentry *dentry, struct nameidata *nd, void *ptr)* @crypt_stat: Crypt_stat associated with file * @upper_size: Size of the upper file * -* Calculate the requried size of the lower file based on the +* Calculate the required size of the lower file based on the * specified size of the upper file. This calculation is based on the * number of headers in the underlying file and the extent size. *
@@ -709,8 +708,7 @@ upper_size_to_lower_size(struct ecryptfs_crypt_stat *crypt_stat, {loff_t lower_size; - lower_size = (crypt_stat->extent_size - * crypt_stat->num_header_extents_at_front); + lower_size = crypt_stat->num_header_bytes_at_front;if (upper_size !=0 ) {loff_t num_extents;
fs/ecryptfs/main.c +0 -5
@@ -650,11 +650,6 @@ static struct ecryptfs_cache_info { .size = sizeof(struct ecryptfs_sb_info), }, { - .cache = &ecryptfs_header_cache_0, - .name = "ecryptfs_headers_0", - .size = PAGE_CACHE_SIZE, - }, - { .cache = &ecryptfs_header_cache_1, .name = "ecryptfs_headers_1", .size = PAGE_CACHE_SIZE,
fs/ecryptfs/mmap.c +12 -9
@@ -102,13 +102,14 @@ static void set_header_info(char *page_virt, struct ecryptfs_crypt_stat *crypt_stat) {size_t written; -int save_num_header_extents_at_front = - crypt_stat->num_header_extents_at_front; +size_t save_num_header_bytes_at_front = + crypt_stat->num_header_bytes_at_front; - crypt_stat->num_header_extents_at_front =1 ; + crypt_stat->num_header_bytes_at_front = +ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE ;ecryptfs_write_header_metadata (page_virt +20 , crypt_stat, &written); - crypt_stat->num_header_extents_at_front = - save_num_header_extents_at_front; + crypt_stat->num_header_bytes_at_front = + save_num_header_bytes_at_front; }/**
@@ -134,8 +135,11 @@ ecryptfs_copy_up_encrypted_with_header(struct page *page,loff_t view_extent_num = ((((loff_t )page->index) * num_extents_per_page) + extent_num_in_page); +size_t num_header_extents_at_front = + (crypt_stat->num_header_bytes_at_front + / crypt_stat->extent_size); -if (view_extent_num < crypt_stat->num_header_extents_at_front) { +if (view_extent_num < num_header_extents_at_front) {/* This is a header extent */ char *page_virt;
@@ -157,9 +161,8 @@ ecryptfs_copy_up_encrypted_with_header(struct page *page, }else {/* This is an encrypted data extent */ loff_t lower_offset = - ((view_extent_num - - crypt_stat->num_header_extents_at_front) - * crypt_stat->extent_size); + ((view_extent_num * crypt_stat->extent_size) + - crypt_stat->num_header_bytes_at_front); rc = ecryptfs_read_lower_page_segment( page, (lower_offset >> PAGE_CACHE_SHIFT),
Signature · 1 line
1.5.0.6
PATCH 2/3 eCryptfs: Increment extent_offset once per loop interation
In reply to Michael HalcrowThe extent_offset is getting incremented twice per loop iteration through any given page. It should only be getting incremented once. This bug should only impact hosts with >4K page sizes.
Signed-off-by Michael Halcrow <[email protected]>
--- fs/ecryptfs/crypto.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
fs/ecryptfs/crypto.c +0 -2
@@ -503,7 +503,6 @@ int ecryptfs_encrypt_page(struct page *page) "\n", rc); goto out; } - extent_offset++; } out:kfree (enc_extent_virt);
@@ -639,7 +638,6 @@ int ecryptfs_decrypt_page(struct page *page) "rc = [%d]\n", __FUNCTION__, rc); goto out; } - extent_offset++; } out:kfree (enc_extent_virt);
Signature · 1 line
1.5.0.6
PATCH 3/3 eCryptfs: Set inode key only once per crypto operation
In reply to Michael HalcrowThere is no need to keep re-setting the same key for any given eCryptfs inode. This patch optimizes the use of the crypto API and helps performance a bit.
Signed-off-by Trevor Highland <[email protected]>
Signed-off-by Michael Halcrow <[email protected]>
--- fs/ecryptfs/crypto.c | 7 +++++-- fs/ecryptfs/ecryptfs_kernel.h | 1 + 2 files changed, 6 insertions(+), 2 deletions(-)
fs/ecryptfs/crypto.c +5 -2
@@ -337,8 +337,11 @@ static int encrypt_scatterlist(struct ecryptfs_crypt_stat *crypt_stat, }/* Consider doing this once, when the file is opened */ mutex_lock (&crypt_stat->cs_tfm_mutex); - rc =crypto_blkcipher_setkey (crypt_stat->tfm, crypt_stat->key, - crypt_stat->key_size); +if (!(crypt_stat->flags &ECRYPTFS_KEY_SET )) { + rc =crypto_blkcipher_setkey (crypt_stat->tfm, crypt_stat->key, + crypt_stat->key_size); + crypt_stat->flags |=ECRYPTFS_KEY_SET ; + }if (rc) {ecryptfs_printk (KERN_ERR ,"Error setting key; rc = [%d]\n" , rc);
fs/ecryptfs/ecryptfs_kernel.h +1 -0
@@ -234,6 +234,7 @@ struct ecryptfs_crypt_stat {#define ECRYPTFS_KEY_VALID 0x00000080#define ECRYPTFS_METADATA_IN_XATTR 0x00000100#define ECRYPTFS_VIEW_AS_ENCRYPTED 0x00000200 +#define ECRYPTFS_KEY_SET 0x00000400u32 flags;unsigned int file_version;size_t iv_bytes;
Signature · 1 line
1.5.0.6
Re: [PATCH 3/3] eCryptfs: Set inode key only once per crypto operation
Quoted 6 lines · On Fri, 2 Nov 2007 13:53:08 -0500 Michael Halcrow <[email protected]> wrote:
There is no need to keep re-setting the same key for any given eCryptfs inode. This patch optimizes the use of the crypto API and helps performance a bit.
Signed-off-by Trevor Highland <[email protected]>Signed-off-by Michael Halcrow <[email protected]>
I've inferred from this that Michael authored this patch. Please let me know if that was incorrect.
The way in which we indicate authorship is to put a From: line right at the top of the changelog. If no such line appears then the From:
line from the mail headers is used.
Re: [PATCH 3/3] eCryptfs: Set inode key only once per crypto operation
Quoted 7 lines · Andrew Morton
Quoted 4 lines · On Fri, 2 Nov 2007 13:53:08 -0500 Michael Halcrow <[email protected]> wrote:
There is no need to keep re-setting the same key for any given eCryptfs inode. This patch optimizes the use of the crypto API and helps performance a bit.
Quoted 5 lines
Quoted 2 lines
Signed-off-by Trevor Highland <[email protected]>Signed-off-by Michael Halcrow <[email protected]>I've inferred from this that Michael authored this patch. Please let me know if that was incorrect.
Trevor Highland actually authored this one.
Quoted 3 lines
The way in which we indicate authorship is to put a From: line right at the top of the changelog. If no such line appears then the From:
line from the mail headers is used.
Noted.
Mike