Re: [Ecryptfs-devel] [PATCH 3/11] eCryptfs: read_write.c routines
On Fri, Sep 21, 2007 at 03:05:40PM -0700, Andrew Morton wrote: btw, I'm not really a great admirer of the whole patchset: it does some pretty nasty-looking things: allocating dynamic memory, grabbing the underlying pageframes with virt_to_page(), passing them back into kernel APIs which are supposed to be called from userspace, etc. It's all rather ugly and abusive-looking. Functions higher up the execution stack should be the ones mucking with the Uptodate flag. The patch below addresses some of these issues. I also whipped up a post-patch partial call graph to help illustrate what is going on with the page mapping and Uptodate status in the various eCryptfs read/write paths: http://ecryptfs.sourceforge.net/ecryptfs-pageuptodate-call-graph.png --- The functions that eventually call down to ecryptfs_read_lower(), ecryptfs_decrypt_page(), and ecryptfs_copy_up_encrypted_with_header() should have the responsibility of managing the page Uptodate status. This patch gets rid of some of the ugliness that resulted from trying to push some of the page flag setting too far down the stack. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index b3795f6..bbec711 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -605,14 +605,14 @@ int ecryptfs_decrypt_page(struct page *page) printk(KERN_ERR %s: Error attempting to copy page at index [%ld]\n, __FUNCTION__, page-index); - goto out_clear_uptodate; + goto out; } enc_extent_virt = kmalloc(PAGE_CACHE_SIZE, GFP_USER); if (!enc_extent_virt) { rc = -ENOMEM; ecryptfs_printk(KERN_ERR, Error allocating memory for encrypted extent\n); - goto out_clear_uptodate; + goto out; } enc_extent_page = virt_to_page(enc_extent_virt); for (extent_offset = 0; @@ -631,21 +631,17 @@ int ecryptfs_decrypt_page(struct page *page) ecryptfs_printk(KERN_ERR, Error attempting to read lower page; rc = [%d] \n, rc); - goto out_clear_uptodate; + goto out; } rc = ecryptfs_decrypt_extent(page, crypt_stat, enc_extent_page, extent_offset); if (rc) { printk(KERN_ERR %s: Error encrypting extent; rc = [%d]\n, __FUNCTION__, rc); - goto out_clear_uptodate; + goto out; } extent_offset++; } - SetPageUptodate(page); - goto out; -out_clear_uptodate: - ClearPageUptodate(page); out: kfree(enc_extent_virt); return rc; diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index bb92b74..ce7a5d4 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -648,6 +648,6 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs, struct inode *ecryptfs_inode); int ecryptfs_read(char *data, loff_t offset, size_t size, struct file *ecryptfs_file); -struct page *ecryptfs_get1page(struct file *file, loff_t index); +struct page *ecryptfs_get_locked_page(struct file *file, loff_t index); #endif /* #ifndef ECRYPTFS_KERNEL_H */ diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 4eb09c1..16a7a55 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -37,23 +37,27 @@ struct kmem_cache *ecryptfs_lower_page_cache; /** - * ecryptfs_get1page + * ecryptfs_get_locked_page * * Get one page from cache or lower f/s, return error otherwise. * - * Returns unlocked and up-to-date page (if ok), with increased + * Returns locked and up-to-date page (if ok), with increased * refcnt. */ -struct page *ecryptfs_get1page(struct file *file, loff_t index) +struct page *ecryptfs_get_locked_page(struct file *file, loff_t index) { struct dentry *dentry; struct inode *inode; struct address_space *mapping; + struct page *page; dentry = file-f_path.dentry; inode = dentry-d_inode; mapping = inode-i_mapping; - return read_mapping_page(mapping, index, (void *)file); + page = read_mapping_page(mapping, index, (void *)file); + if (!IS_ERR(page)) + lock_page(page); + return page; } /** @@ -146,12 +150,10 @@ ecryptfs_copy_up_encrypted_with_header(struct page *page, kunmap_atomic(page_virt, KM_USER0); flush_dcache_page(page); if (rc) { - ClearPageUptodate(page
Re: [PATCH 3/11] eCryptfs: read_write.c routines
On Wed, Sep 19, 2007 at 10:38:50PM -0700, Andrew Morton wrote: + offset = (page_for_lower-index PAGE_CACHE_SHIFT) + offset_in_page; bug. You need to cast page.index to loff_t before shifting. I'd fix it on the spot, but this would be a good time to review the whole patchset and perhaps the whole fs for this easy-to-do, hard-to-find bug. Update data types and add casts in order to avoid potential overflow issues. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 5d27cf9..4bf1a95 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -149,7 +149,7 @@ out: * ecryptfs_derive_iv * @iv: destination for the derived iv vale * @crypt_stat: Pointer to crypt_stat struct for the current inode - * @offset: Offset of the page whose's iv we are to derive + * @offset: Offset of the extent whose IV we are to derive * * Generate the initialization vector from the given root IV and page * offset. @@ -157,7 +157,7 @@ out: * Returns zero on success; non-zero on error. */ static int ecryptfs_derive_iv(char *iv, struct ecryptfs_crypt_stat *crypt_stat, - pgoff_t offset) + loff_t offset) { int rc = 0; char dst[MD5_DIGEST_SIZE]; @@ -173,7 +173,7 @@ static int ecryptfs_derive_iv(char *iv, struct ecryptfs_crypt_stat *crypt_stat, * hashing business. -Halcrow */ memcpy(src, crypt_stat-root_iv, crypt_stat-iv_bytes); memset((src + crypt_stat-iv_bytes), 0, 16); - snprintf((src + crypt_stat-iv_bytes), 16, %ld, offset); + snprintf((src + crypt_stat-iv_bytes), 16, %lld, offset); if (unlikely(ecryptfs_verbosity 0)) { ecryptfs_printk(KERN_DEBUG, source:\n); ecryptfs_dump_hex(src, (crypt_stat-iv_bytes + 16)); @@ -384,11 +384,11 @@ static int ecryptfs_encrypt_extent(struct page *enc_extent_page, struct page *page, unsigned long extent_offset) { - unsigned long extent_base; + loff_t extent_base; char extent_iv[ECRYPTFS_MAX_IV_BYTES]; int rc; - extent_base = (page-index + extent_base = (((loff_t)page-index) * (PAGE_CACHE_SIZE / crypt_stat-extent_size)); rc = ecryptfs_derive_iv(extent_iv, crypt_stat, (extent_base + extent_offset)); @@ -492,8 +492,9 @@ int ecryptfs_encrypt_page(struct page *page) goto out; } ecryptfs_lower_offset_for_extent( - offset, ((page-index * (PAGE_CACHE_SIZE - / crypt_stat-extent_size)) + offset, loff_t)page-index) + * (PAGE_CACHE_SIZE + / crypt_stat-extent_size)) + extent_offset), crypt_stat); rc = ecryptfs_write_lower(ecryptfs_inode, enc_extent_virt, offset, crypt_stat-extent_size); @@ -515,11 +516,11 @@ static int ecryptfs_decrypt_extent(struct page *page, struct page *enc_extent_page, unsigned long extent_offset) { - unsigned long extent_base; + loff_t extent_base; char extent_iv[ECRYPTFS_MAX_IV_BYTES]; int rc; - extent_base = (page-index + extent_base = (((loff_t)page-index) * (PAGE_CACHE_SIZE / crypt_stat-extent_size)); rc = ecryptfs_derive_iv(extent_iv, crypt_stat, (extent_base + extent_offset)); @@ -1320,7 +1321,7 @@ ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat, while (current_header_page header_pages) { loff_t offset; - offset = (current_header_page PAGE_CACHE_SHIFT); + offset = (((loff_t)current_header_page) PAGE_CACHE_SHIFT); if ((rc = ecryptfs_write_lower(ecryptfs_dentry-d_inode, page_virt, offset, PAGE_CACHE_SIZE))) { diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index c6a8a33..4eb09c1 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -127,7 +127,8 @@ ecryptfs_copy_up_encrypted_with_header(struct page *page, int rc = 0; while (extent_num_in_page num_extents_per_page) { - loff_t view_extent_num = ((page-index * num_extents_per_page) + loff_t view_extent_num = loff_t)page-index) + * num_extents_per_page) + extent_num_in_page); if (view_extent_num crypt_stat-num_header_extents_at_front) { @@ -418,7 +419,7 @@ static int ecryptfs_commit_write
Re: [PATCH 6/11] eCryptfs: Update metadata read/write functions
On Wed, Sep 19, 2007 at 10:48:17PM -0700, Andrew Morton wrote: On Mon, 17 Sep 2007 16:48:44 -0500 Michael Halcrow [EMAIL PROTECTED] wrote: + if ((rc = ecryptfs_write_lower(ecryptfs_dentry-d_inode, checkpatch missed the assignment-in-an-if here. Fix an assignment-in-an-if. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 4bf1a95..b3795f6 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1306,8 +1306,9 @@ ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat, int header_pages; int rc; - if ((rc = ecryptfs_write_lower(ecryptfs_dentry-d_inode, page_virt, - 0, PAGE_CACHE_SIZE))) { + rc = ecryptfs_write_lower(ecryptfs_dentry-d_inode, page_virt, + 0, PAGE_CACHE_SIZE); + if (rc) { printk(KERN_ERR %s: Error attempting to write header information to lower file; rc = [%d]\n, __FUNCTION__, rc); - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Ecryptfs-devel] [PATCH 3/11] eCryptfs: read_write.c routines
On Wed, Sep 19, 2007 at 10:38:50PM -0700, Andrew Morton wrote: + virt = kmap(page_for_lower); + rc = ecryptfs_write_lower(ecryptfs_inode, virt, offset, size); + kunmap(page_for_lower); + return rc; +} argh, kmap. http://lkml.org/lkml/2007/9/15/55 Here is a patch that moves to kmap_atomic(), adding an intermediate copy. Although I would really like to find a way to avoid having to do this extra copy. --- Replace kmap() with kmap_atomic() for read_write.c routines kmap() can lead to deadlock when multiple tasks attempt to take more than one simultaneously: http://lkml.org/lkml/2007/9/15/55 In order to avoid this possibility, eCryptfs must allocate an intermediate block of memory to use with vfs_read() and vfs_write(), copying the data through this memory region, since kmap_atomic() cannot be held during calls which may block. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index bb92b74..ce7a5d4 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -648,6 +648,6 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs, struct inode *ecryptfs_inode); int ecryptfs_read(char *data, loff_t offset, size_t size, struct file *ecryptfs_file); -struct page *ecryptfs_get1page(struct file *file, loff_t index); +struct page *ecryptfs_get_locked_page(struct file *file, loff_t index); #endif /* #ifndef ECRYPTFS_KERNEL_H */ diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index c6a8a33..6abf805 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -37,23 +37,30 @@ struct kmem_cache *ecryptfs_lower_page_cache; /** - * ecryptfs_get1page + * ecryptfs_get_locked_page * * Get one page from cache or lower f/s, return error otherwise. * - * Returns unlocked and up-to-date page (if ok), with increased + * Returns a locked and up-to-date page (if ok), with increased * refcnt. */ -struct page *ecryptfs_get1page(struct file *file, loff_t index) +struct page *ecryptfs_get_locked_page(struct file *file, loff_t index) { struct dentry *dentry; struct inode *inode; struct address_space *mapping; + struct page *page; dentry = file-f_path.dentry; inode = dentry-d_inode; mapping = inode-i_mapping; - return read_mapping_page(mapping, index, (void *)file); + page = read_mapping_page(mapping, index, (void *)file); + if (!IS_ERR(page)) + lock_page(page); + else + printk(KERN_ERR %s: Error from read_mapping_page()\n, + __FUNCTION__); + return page; } /** diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c index ccd2599..6732a4c 100644 --- a/fs/ecryptfs/read_write.c +++ b/fs/ecryptfs/read_write.c @@ -83,14 +83,24 @@ int ecryptfs_write_lower_page_segment(struct inode *ecryptfs_inode, struct page *page_for_lower, size_t offset_in_page, size_t size) { - char *virt; + char *page_for_lower_virt; + char *tmp_virt; loff_t offset; int rc; - offset = (page_for_lower-index PAGE_CACHE_SHIFT) + offset_in_page; - virt = kmap(page_for_lower); - rc = ecryptfs_write_lower(ecryptfs_inode, virt, offset, size); - kunmap(page_for_lower); + tmp_virt = kmalloc(PAGE_CACHE_SIZE, GFP_KERNEL); + if (!tmp_virt) { + rc = -ENOMEM; + goto out; + } + offset = loff_t)page_for_lower-index) PAGE_CACHE_SHIFT) + + offset_in_page); + page_for_lower_virt = kmap_atomic(page_for_lower, KM_USER0); + memcpy(tmp_virt, page_for_lower_virt, PAGE_CACHE_SIZE); + kunmap_atomic(page_for_lower_virt, KM_USER0); + rc = ecryptfs_write_lower(ecryptfs_inode, tmp_virt, offset, size); + kfree(tmp_virt); +out: return rc; } @@ -140,8 +150,8 @@ int ecryptfs_write(struct file *ecryptfs_file, char *data, loff_t offset, if (num_bytes total_remaining_zeros) num_bytes = total_remaining_zeros; } - ecryptfs_page = ecryptfs_get1page(ecryptfs_file, - ecryptfs_page_idx); + ecryptfs_page = ecryptfs_get_locked_page(ecryptfs_file, +ecryptfs_page_idx); if (IS_ERR(ecryptfs_page)) { rc = PTR_ERR(ecryptfs_page); printk(KERN_ERR %s: Error getting page at @@ -159,6 +169,7 @@ int ecryptfs_write(struct file *ecryptfs_file, char *data, loff_t offset, printk(KERN_ERR %s: Error decrypting page; rc = [%d]\n, __FUNCTION__, rc
Re: [PATCH 4/11] eCryptfs: Replace encrypt, decrypt, and inode size write
On Wed, Sep 19, 2007 at 10:46:26PM -0700, Andrew Morton wrote: (from ecryptfs_encrypt_page()): + enc_extent_virt = kmalloc(PAGE_CACHE_SIZE, GFP_USER); I'd have thought that alloc_page() would be nicer. After all, we _are_ treating it as a page, and not as a random piece of memry. + if (!enc_extent_virt) { + rc = -ENOMEM; + ecryptfs_printk(KERN_ERR, Error allocating memory for + encrypted extent\n); + goto out; + } + enc_extent_page = virt_to_page(enc_extent_virt); And then we don't need this. If neither kmap() nor kmap_atomic() can be safely used to get a virtual address to pass to vfs_write(), then I do not know what my other options are here. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 8/11] eCryptfs: Convert mmap functions to use persistent file
On Wed, Sep 19, 2007 at 10:50:57PM -0700, Andrew Morton wrote: On Mon, 17 Sep 2007 16:50:16 -0500 Michael Halcrow [EMAIL PROTECTED] wrote: +ecryptfs_copy_up_encrypted_with_header(struct page *page, + struct ecryptfs_crypt_stat *crypt_stat) +{ ... + flush_dcache_page(page); + if (rc) { + ClearPageUptodate(page); + printk(KERN_ERR %s: Error reading xattr + region; rc = [%d]\n, __FUNCTION__, rc); + goto out; + } + SetPageUptodate(page); I don't know what sort of page `page' refers to here, but normally we only manipulate the page uptodate status under lock_page(). This is the page that eCryptfs gets via ecryptfs_aops-ecryptfs_readpage(), so this should be okay. The comment should make the fact that the page is locked explicit. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 04103ff..c6a8a33 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -111,7 +111,7 @@ static void set_header_info(char *page_virt, * ecryptfs_copy_up_encrypted_with_header * @page: Sort of a ``virtual'' representation of the encrypted lower *file. The actual lower file does not have the metadata in - *the header. + *the header. This is locked. * @crypt_stat: The eCryptfs inode's cryptographic context * * The ``view'' is the version of the file that userspace winds up - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/11] eCryptfs: Introduce persistent lower files for each eCryptfs inode
Currently, eCryptfs directly accesses the lower inode address space, doing things like grab_cache_page() on lower_inode-i_mapping. It really should not do that. The main point of this patch set is to make all I/O with the lower files go through vfs_read() and vfs_write() instead. In order to accomplish this, eCryptfs needs a way to call vfs_read() and vfs_write() on the lower file when ecryptfs_aops-readpage() and ecryptfs_aops-writepage() are called. I propose keeping a persistent lower file around for each eCryptfs inode. This is the only lower file that eCryptfs will open for any given eCryptfs inode; multiple eCryptfs files may map to this one persistent lower file. When the eCrypfs inode is destroyed, this persistent lower file is closed. Consolidating all reads and writes to the lower file to a single execution path simplifies the code. This should also make it easier to port eCryptfs to use the asynchronous crypto API functions. Note that this patch set also removes all direct calls to lower prepare_write() and commite_write(), fixing an oops when mounted on NFS. Mike - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/11] eCryptfs: Remove header_extent_size
There is no point to keeping a separate header_extent_size and an extent_size. The total size of the header can always be represented as some multiple of the regular data extent size. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c | 40 fs/ecryptfs/ecryptfs_kernel.h | 39 +++ fs/ecryptfs/inode.c |7 --- fs/ecryptfs/mmap.c|2 +- 4 files changed, 52 insertions(+), 36 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 8e9b36d..3dbb21a 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -366,8 +366,8 @@ ecryptfs_extent_to_lwr_pg_idx_and_offset(unsigned long *lower_page_idx, int extents_per_page; bytes_occupied_by_headers_at_front = - ( crypt_stat-header_extent_size - * crypt_stat-num_header_extents_at_front ); + (crypt_stat-extent_size +* crypt_stat-num_header_extents_at_front); extents_occupied_by_headers_at_front = ( bytes_occupied_by_headers_at_front / crypt_stat-extent_size ); @@ -376,8 +376,8 @@ ecryptfs_extent_to_lwr_pg_idx_and_offset(unsigned long *lower_page_idx, (*lower_page_idx) = lower_extent_num / extents_per_page; extent_offset = lower_extent_num % extents_per_page; (*byte_offset) = extent_offset * crypt_stat-extent_size; - ecryptfs_printk(KERN_DEBUG, * crypt_stat-header_extent_size = - [%d]\n, crypt_stat-header_extent_size); + ecryptfs_printk(KERN_DEBUG, * crypt_stat-extent_size = + [%d]\n, crypt_stat-extent_size); ecryptfs_printk(KERN_DEBUG, * crypt_stat- num_header_extents_at_front = [%d]\n, crypt_stat-num_header_extents_at_front); @@ -899,15 +899,17 @@ void ecryptfs_set_default_sizes(struct ecryptfs_crypt_stat *crypt_stat) crypt_stat-extent_size = ECRYPTFS_DEFAULT_EXTENT_SIZE; set_extent_mask_and_shift(crypt_stat); crypt_stat-iv_bytes = ECRYPTFS_DEFAULT_IV_BYTES; - if (PAGE_CACHE_SIZE = ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE) { - crypt_stat-header_extent_size = - ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE; - } else - crypt_stat-header_extent_size = PAGE_CACHE_SIZE; if (crypt_stat-flags ECRYPTFS_METADATA_IN_XATTR) crypt_stat-num_header_extents_at_front = 0; - else - crypt_stat-num_header_extents_at_front = 1; + 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); + else + crypt_stat-num_header_extents_at_front = + (PAGE_CACHE_SIZE / crypt_stat-extent_size); + } } /** @@ -1319,7 +1321,7 @@ ecryptfs_write_header_metadata(char *virt, u32 header_extent_size; u16 num_header_extents_at_front; - header_extent_size = (u32)crypt_stat-header_extent_size; + header_extent_size = (u32)crypt_stat-extent_size; num_header_extents_at_front = (u16)crypt_stat-num_header_extents_at_front; header_extent_size = cpu_to_be32(header_extent_size); @@ -1415,7 +1417,7 @@ ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat, set_fs(oldfs); goto out; } - header_pages = ((crypt_stat-header_extent_size + header_pages = ((crypt_stat-extent_size * crypt_stat-num_header_extents_at_front) / PAGE_CACHE_SIZE); memset(page_virt, 0, PAGE_CACHE_SIZE); @@ -1532,17 +1534,16 @@ static int parse_header_metadata(struct ecryptfs_crypt_stat *crypt_stat, virt += 4; memcpy(num_header_extents_at_front, virt, 2); num_header_extents_at_front = be16_to_cpu(num_header_extents_at_front); - crypt_stat-header_extent_size = (int)header_extent_size; crypt_stat-num_header_extents_at_front = (int)num_header_extents_at_front; - (*bytes_read) = 6; + (*bytes_read) = (sizeof(u32) + sizeof(u16)); if ((validate_header_size == ECRYPTFS_VALIDATE_HEADER_SIZE) -((crypt_stat-header_extent_size +((crypt_stat-extent_size * crypt_stat-num_header_extents_at_front) ECRYPTFS_MINIMUM_HEADER_EXTENT_SIZE)) { rc = -EINVAL; - ecryptfs_printk(KERN_WARNING, Invalid header extent size: - [%d]\n, crypt_stat-header_extent_size); + printk(KERN_WARNING Invalid number of header extents: [%d]\n
[PATCH 2/11] eCryptfs: Remove assignments in if-statements
Remove assignments in if-statements. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c| 17 -- fs/ecryptfs/file.c |8 -- fs/ecryptfs/inode.c | 35 ++ fs/ecryptfs/keystore.c | 55 +- fs/ecryptfs/main.c | 28 ++- fs/ecryptfs/messaging.c |5 ++- fs/ecryptfs/mmap.c |5 ++- 7 files changed, 89 insertions(+), 64 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 3dbb21a..5d8a553 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1277,8 +1277,8 @@ static int ecryptfs_read_header_region(char *data, struct dentry *dentry, mm_segment_t oldfs; int rc; - if ((rc = ecryptfs_open_lower_file(lower_file, dentry, mnt, - O_RDONLY))) { + rc = ecryptfs_open_lower_file(lower_file, dentry, mnt, O_RDONLY); + if (rc) { printk(KERN_ERR Error opening lower_file to read header region\n); goto out; @@ -1289,7 +1289,8 @@ static int ecryptfs_read_header_region(char *data, struct dentry *dentry, rc = lower_file-f_op-read(lower_file, (char __user *)data, ECRYPTFS_DEFAULT_EXTENT_SIZE, lower_file-f_pos); set_fs(oldfs); - if ((rc = ecryptfs_close_lower_file(lower_file))) { + rc = ecryptfs_close_lower_file(lower_file); + if (rc) { printk(KERN_ERR Error closing lower_file\n); goto out; } @@ -1951,9 +1952,10 @@ ecryptfs_add_new_key_tfm(struct ecryptfs_key_tfm **key_tfm, char *cipher_name, strncpy(tmp_tfm-cipher_name, cipher_name, ECRYPTFS_MAX_CIPHER_NAME_SIZE); tmp_tfm-key_size = key_size; - if ((rc = ecryptfs_process_key_cipher(tmp_tfm-key_tfm, - tmp_tfm-cipher_name, - tmp_tfm-key_size))) { + rc = ecryptfs_process_key_cipher(tmp_tfm-key_tfm, +tmp_tfm-cipher_name, +tmp_tfm-key_size); + if (rc) { printk(KERN_ERR Error attempting to initialize key TFM cipher with name = [%s]; rc = [%d]\n, tmp_tfm-cipher_name, rc); @@ -1988,7 +1990,8 @@ int ecryptfs_get_tfm_and_mutex_for_cipher_name(struct crypto_blkcipher **tfm, } } mutex_unlock(key_tfm_list_mutex); - if ((rc = ecryptfs_add_new_key_tfm(key_tfm, cipher_name, 0))) { + rc = ecryptfs_add_new_key_tfm(key_tfm, cipher_name, 0); + if (rc) { printk(KERN_ERR Error adding new key_tfm to list; rc = [%d]\n, rc); goto out; diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index 12ba7e3..59c846d 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -230,8 +230,9 @@ static int ecryptfs_open(struct inode *inode, struct file *file) lower_flags = ~O_APPEND; lower_mnt = ecryptfs_dentry_to_lower_mnt(ecryptfs_dentry); /* Corresponding fput() in ecryptfs_release() */ - if ((rc = ecryptfs_open_lower_file(lower_file, lower_dentry, lower_mnt, - lower_flags))) { + rc = ecryptfs_open_lower_file(lower_file, lower_dentry, lower_mnt, + lower_flags); + if (rc) { ecryptfs_printk(KERN_ERR, Error opening lower file\n); goto out_puts; } @@ -300,7 +301,8 @@ static int ecryptfs_release(struct inode *inode, struct file *file) struct inode *lower_inode = ecryptfs_inode_to_lower(inode); int rc; - if ((rc = ecryptfs_close_lower_file(lower_file))) { + rc = ecryptfs_close_lower_file(lower_file); + if (rc) { printk(KERN_ERR Error closing lower_file\n); goto out; } diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index abac91c..d70f599 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -202,8 +202,9 @@ static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry) lower_flags = ((O_CREAT | O_TRUNC) O_ACCMODE) | O_RDWR; lower_mnt = ecryptfs_dentry_to_lower_mnt(ecryptfs_dentry); /* Corresponding fput() at end of this function */ - if ((rc = ecryptfs_open_lower_file(lower_file, lower_dentry, lower_mnt, - lower_flags))) { + rc = ecryptfs_open_lower_file(lower_file, lower_dentry, lower_mnt, + lower_flags); + if (rc) { ecryptfs_printk(KERN_ERR, Error opening dentry; rc = [%i]\n, rc); goto out; @@ -229,7 +230,8 @@ static int ecryptfs_initialize_file(struct dentry
[PATCH 3/11] eCryptfs: read_write.c routines
Add a set of functions through which all I/O to lower files is consolidated. This patch adds a new inode_info reference to a persistent lower file for each eCryptfs inode; another patch later in this series will set that up. This persistent lower file is what the read_write.c functions use to call vfs_read() and vfs_write() on the lower filesystem, so even when reads and writes come in through aops-readpage and aops-writepage, we can satisfy them without resorting to direct access to the lower inode's address space. Several function declarations are going to be changing with this patchset. For now, in order to keep from breaking the build, I am putting dummy parameters in for those functions. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/Makefile |2 +- fs/ecryptfs/ecryptfs_kernel.h | 18 ++ fs/ecryptfs/mmap.c|2 +- fs/ecryptfs/read_write.c | 359 + 4 files changed, 379 insertions(+), 2 deletions(-) create mode 100644 fs/ecryptfs/read_write.c diff --git a/fs/ecryptfs/Makefile b/fs/ecryptfs/Makefile index 1f11072..7688570 100644 --- a/fs/ecryptfs/Makefile +++ b/fs/ecryptfs/Makefile @@ -4,4 +4,4 @@ obj-$(CONFIG_ECRYPT_FS) += ecryptfs.o -ecryptfs-objs := dentry.o file.o inode.o main.o super.o mmap.o crypto.o keystore.o messaging.o netlink.o debug.o +ecryptfs-objs := dentry.o file.o inode.o main.o super.o mmap.o read_write.o crypto.o keystore.o messaging.o netlink.o debug.o diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index a618ab7..e6a68a8 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h @@ -260,6 +260,8 @@ struct ecryptfs_crypt_stat { struct ecryptfs_inode_info { struct inode vfs_inode; struct inode *wii_inode; + struct file *lower_file; + struct mutex lower_file_mutex; struct ecryptfs_crypt_stat crypt_stat; }; @@ -653,5 +655,21 @@ int ecryptfs_keyring_auth_tok_for_sig(struct key **auth_tok_key, char *sig); int ecryptfs_write_zeros(struct file *file, pgoff_t index, int start, int num_zeros); +int ecryptfs_write_lower(struct inode *ecryptfs_inode, char *data, +loff_t offset, size_t size); +int ecryptfs_write_lower_page_segment(struct inode *ecryptfs_inode, + struct page *page_for_lower, + size_t offset_in_page, size_t size); +int ecryptfs_write(struct file *ecryptfs_file, char *data, loff_t offset, + size_t size); +int ecryptfs_read_lower(char *data, loff_t offset, size_t size, + struct inode *ecryptfs_inode); +int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs, +pgoff_t page_index, +size_t offset_in_page, size_t size, +struct inode *ecryptfs_inode); +int ecryptfs_read(char *data, loff_t offset, size_t size, + struct file *ecryptfs_file); +struct page *ecryptfs_get1page(struct file *file, loff_t index); #endif /* #ifndef ECRYPTFS_KERNEL_H */ diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 307f7ee..0c53320 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -44,7 +44,7 @@ struct kmem_cache *ecryptfs_lower_page_cache; * Returns unlocked and up-to-date page (if ok), with increased * refcnt. */ -static struct page *ecryptfs_get1page(struct file *file, int index) +struct page *ecryptfs_get1page(struct file *file, loff_t index) { struct dentry *dentry; struct inode *inode; diff --git a/fs/ecryptfs/read_write.c b/fs/ecryptfs/read_write.c new file mode 100644 index 000..e59c94a --- /dev/null +++ b/fs/ecryptfs/read_write.c @@ -0,0 +1,359 @@ +/** + * eCryptfs: Linux filesystem encryption layer + * + * Copyright (C) 2007 International Business Machines Corp. + * Author(s): Michael A. Halcrow [EMAIL PROTECTED] + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA + * 02111-1307, USA. + */ + +#include linux/fs.h +#include linux/pagemap.h +#include ecryptfs_kernel.h + +/** + * ecryptfs_write_lower + * @ecryptfs_inode: The eCryptfs inode + * @data: Data to write + * @offset: Byte offset
[PATCH 4/11] eCryptfs: Replace encrypt, decrypt, and inode size write
Replace page encryption and decryption routines and inode size write routine with versions that utilize the read_write.c functions. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c | 427 ++-- fs/ecryptfs/ecryptfs_kernel.h | 14 +- fs/ecryptfs/inode.c | 12 +- fs/ecryptfs/mmap.c| 131 - fs/ecryptfs/read_write.c | 12 +- 5 files changed, 290 insertions(+), 306 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 5d8a553..b829d3c 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -467,8 +467,91 @@ out: } /** + * ecryptfs_lower_offset_for_extent + * + * Convert an eCryptfs page index into a lower byte offset + */ +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) ++ (crypt_stat-extent_size * extent_num)); +} + +/** + * ecryptfs_encrypt_extent + * @enc_extent_page: Allocated page into which to encrypt the data in + * @page + * @crypt_stat: crypt_stat containing cryptographic context for the + * encryption operation + * @page: Page containing plaintext data extent to encrypt + * @extent_offset: Page extent offset for use in generating IV + * + * Encrypts one extent of data. + * + * Return zero on success; non-zero otherwise + */ +static int ecryptfs_encrypt_extent(struct page *enc_extent_page, + struct ecryptfs_crypt_stat *crypt_stat, + struct page *page, + unsigned long extent_offset) +{ + unsigned long extent_base; + char extent_iv[ECRYPTFS_MAX_IV_BYTES]; + int rc; + + extent_base = (page-index + * (PAGE_CACHE_SIZE / crypt_stat-extent_size)); + rc = ecryptfs_derive_iv(extent_iv, crypt_stat, + (extent_base + extent_offset)); + if (rc) { + ecryptfs_printk(KERN_ERR, Error attempting to + derive IV for extent [0x%.16x]; + rc = [%d]\n, (extent_base + extent_offset), + rc); + goto out; + } + if (unlikely(ecryptfs_verbosity 0)) { + ecryptfs_printk(KERN_DEBUG, Encrypting extent + with iv:\n); + ecryptfs_dump_hex(extent_iv, crypt_stat-iv_bytes); + ecryptfs_printk(KERN_DEBUG, First 8 bytes before + encryption:\n); + ecryptfs_dump_hex((char *) + (page_address(page) + + (extent_offset * crypt_stat-extent_size)), + 8); + } + rc = ecryptfs_encrypt_page_offset(crypt_stat, enc_extent_page, 0, + page, (extent_offset +* crypt_stat-extent_size), + crypt_stat-extent_size, extent_iv); + if (rc 0) { + printk(KERN_ERR %s: Error attempting to encrypt page with + page-index = [%ld], extent_offset = [%ld]; + rc = [%d]\n, __FUNCTION__, page-index, extent_offset, + rc); + goto out; + } + rc = 0; + if (unlikely(ecryptfs_verbosity 0)) { + ecryptfs_printk(KERN_DEBUG, Encrypt extent [0x%.16x]; + rc = [%d]\n, (extent_base + extent_offset), + rc); + ecryptfs_printk(KERN_DEBUG, First 8 bytes after + encryption:\n); + ecryptfs_dump_hex((char *)(page_address(enc_extent_page)), 8); + } +out: + return rc; +} + +/** * ecryptfs_encrypt_page - * @ctx: The context of the page + * @page: Page mapped from the eCryptfs inode for the file; contains + *decrypted content that needs to be encrypted (to a temporary + *page; not in place) and written out to the lower file * * Encrypt an eCryptfs page. This is done on a per-extent basis. Note * that eCryptfs pages may straddle the lower pages -- for instance, @@ -478,128 +561,121 @@ out: * file, 24K of page 0 of the lower file will be read and decrypted, * and then 8K of page 1 of the lower file will be read and decrypted. * - * The actual operations performed on each page depends on the - * contents of the ecryptfs_page_crypt_context struct. - * * Returns zero on success; negative on error */ -int ecryptfs_encrypt_page(struct ecryptfs_page_crypt_context *ctx) +int ecryptfs_encrypt_page(struct page *page) { - char extent_iv
[PATCH 5/11] eCryptfs: Set up and destroy persistent lower file
This patch sets up and destroys the persistent lower file for each eCryptfs inode. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/inode.c | 23 +++--- fs/ecryptfs/main.c | 65 +++ fs/ecryptfs/super.c | 22 +++-- 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 7192a81..c746b5d 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -119,10 +119,23 @@ ecryptfs_do_create(struct inode *directory_inode, } rc = ecryptfs_create_underlying_file(lower_dir_dentry-d_inode, ecryptfs_dentry, mode, nd); - if (unlikely(rc)) { - ecryptfs_printk(KERN_ERR, - Failure to create underlying file\n); - goto out_lock; + if (rc) { + struct inode *ecryptfs_inode = ecryptfs_dentry-d_inode; + struct ecryptfs_inode_info *inode_info = + ecryptfs_inode_to_private(ecryptfs_inode); + + printk(KERN_WARNING %s: Error creating underlying file; + rc = [%d]; checking for existing\n, __FUNCTION__, rc); + if (inode_info) { + mutex_lock(inode_info-lower_file_mutex); + if (!inode_info-lower_file) { + mutex_unlock(inode_info-lower_file_mutex); + printk(KERN_ERR %s: Failure to set underlying + file; rc = [%d]\n, __FUNCTION__, rc); + goto out_lock; + } + mutex_unlock(inode_info-lower_file_mutex); + } } rc = ecryptfs_interpose(lower_dentry, ecryptfs_dentry, directory_inode-i_sb, 0); @@ -252,6 +265,8 @@ ecryptfs_create(struct inode *directory_inode, struct dentry *ecryptfs_dentry, { int rc; + /* ecryptfs_do_create() calls ecryptfs_interpose(), which opens +* the crypt_stat-lower_file (persistent file) */ rc = ecryptfs_do_create(directory_inode, ecryptfs_dentry, mode, nd); if (unlikely(rc)) { ecryptfs_printk(KERN_WARNING, Failed to create file in diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 967bad0..3e324f8 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -98,6 +98,64 @@ void __ecryptfs_printk(const char *fmt, ...) } /** + * ecryptfs_init_persistent_file + * @ecryptfs_dentry: Fully initialized eCryptfs dentry object, with + * the lower dentry and the lower mount set + * + * eCryptfs only ever keeps a single open file for every lower + * inode. All I/O operations to the lower inode occur through that + * file. When the first eCryptfs dentry that interposes with the first + * lower dentry for that inode is created, this function creates the + * persistent file struct and associates it with the eCryptfs + * inode. When the eCryptfs inode is destroyed, the file is closed. + * + * The persistent file will be opened with read/write permissions, if + * possible. Otherwise, it is opened read-only. + * + * This function does nothing if a lower persistent file is already + * associated with the eCryptfs inode. + * + * Returns zero on success; non-zero otherwise + */ +int ecryptfs_init_persistent_file(struct dentry *ecryptfs_dentry) +{ + struct ecryptfs_inode_info *inode_info = + ecryptfs_inode_to_private(ecryptfs_dentry-d_inode); + int rc = 0; + + mutex_lock(inode_info-lower_file_mutex); + if (!inode_info-lower_file) { + struct dentry *lower_dentry; + struct vfsmount *lower_mnt = + ecryptfs_dentry_to_lower_mnt(ecryptfs_dentry); + + lower_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry); + /* Corresponding dput() and mntput() are done when the +* persistent file is fput() when the eCryptfs inode +* is destroyed. */ + dget(lower_dentry); + mntget(lower_mnt); + inode_info-lower_file = dentry_open(lower_dentry, +lower_mnt, +(O_RDWR | O_LARGEFILE)); + if (IS_ERR(inode_info-lower_file)) + inode_info-lower_file = dentry_open(lower_dentry, +lower_mnt, +(O_RDONLY + | O_LARGEFILE)); + if (IS_ERR(inode_info-lower_file)) { + printk(KERN_ERR Error opening lower persistent file + for lower_dentry [0x%p] and lower_mnt [0x%p]\n
[PATCH 6/11] eCryptfs: Update metadata read/write functions
Update the metadata read/write functions and grow_file() to use the read_write.c routines. Do not open another lower file; use the persistent lower file instead. Provide a separate function for crypto.c::ecryptfs_read_xattr_region() to get to the lower xattr without having to go through the eCryptfs getxattr. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c | 126 +++-- fs/ecryptfs/ecryptfs_kernel.h | 15 +++-- fs/ecryptfs/file.c|2 +- fs/ecryptfs/inode.c | 101 +++-- fs/ecryptfs/mmap.c|2 +- 5 files changed, 113 insertions(+), 133 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index d6a0680..6b4d310 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1344,21 +1344,28 @@ out: return rc; } -int ecryptfs_read_and_validate_header_region(char *data, struct dentry *dentry, -struct vfsmount *mnt) +int ecryptfs_read_and_validate_header_region(char *data, +struct inode *ecryptfs_inode) { + struct ecryptfs_crypt_stat *crypt_stat = + (ecryptfs_inode_to_private(ecryptfs_inode)-crypt_stat); int rc; - rc = ecryptfs_read_header_region(data, dentry, mnt); - if (rc) + rc = ecryptfs_read_lower(data, 0, crypt_stat-extent_size, +ecryptfs_inode); + if (rc) { + printk(KERN_ERR %s: Error reading header region; rc = [%d]\n, + __FUNCTION__, rc); goto out; - if (!contains_ecryptfs_marker(data + ECRYPTFS_FILE_SIZE_BYTES)) + } + if (!contains_ecryptfs_marker(data + ECRYPTFS_FILE_SIZE_BYTES)) { rc = -EINVAL; + ecryptfs_printk(KERN_DEBUG, Valid marker not found\n); + } out: return rc; } - void ecryptfs_write_header_metadata(char *virt, struct ecryptfs_crypt_stat *crypt_stat, @@ -1443,24 +1450,18 @@ 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 file *lower_file, char *page_virt) + struct dentry *ecryptfs_dentry, + char *page_virt) { - mm_segment_t oldfs; int current_header_page; int header_pages; - ssize_t size; - int rc = 0; + int rc; - lower_file-f_pos = 0; - oldfs = get_fs(); - set_fs(get_ds()); - size = vfs_write(lower_file, (char __user *)page_virt, PAGE_CACHE_SIZE, -lower_file-f_pos); - if (size 0) { - rc = (int)size; - printk(KERN_ERR Error attempting to write lower page; - rc = [%d]\n, rc); - set_fs(oldfs); + if ((rc = ecryptfs_write_lower(ecryptfs_dentry-d_inode, page_virt, + 0, PAGE_CACHE_SIZE))) { + 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 @@ -1469,18 +1470,19 @@ ecryptfs_write_metadata_to_contents(struct ecryptfs_crypt_stat *crypt_stat, memset(page_virt, 0, PAGE_CACHE_SIZE); current_header_page = 1; while (current_header_page header_pages) { - size = vfs_write(lower_file, (char __user *)page_virt, -PAGE_CACHE_SIZE, lower_file-f_pos); - if (size 0) { - rc = (int)size; - printk(KERN_ERR Error attempting to write lower page; - rc = [%d]\n, rc); - set_fs(oldfs); + loff_t offset; + + offset = (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++; } - set_fs(oldfs); out: return rc; } @@ -1500,7 +1502,6 @@ ecryptfs_write_metadata_to_xattr(struct dentry *ecryptfs_dentry, /** * ecryptfs_write_metadata * @ecryptfs_dentry: The eCryptfs dentry - * @lower_file: The lower file struct, which was returned from dentry_open * * Write the file headers out. This will likely involve
[PATCH 7/11] eCryptfs: Make open, truncate, and setattr use persistent file
Rather than open a new lower file for every eCryptfs file that is opened, truncated, or setattr'd, instead use the existing lower persistent file for the eCryptfs inode. Change truncate to use read_write.c functions. Change ecryptfs_getxattr() to use the common ecryptfs_getxattr_lower() function. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c |2 +- fs/ecryptfs/file.c | 50 -- fs/ecryptfs/inode.c | 113 +++--- 3 files changed, 44 insertions(+), 121 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 6b4d310..b3014d7 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1674,7 +1674,7 @@ out: /** * ecryptfs_read_xattr_region * @page_virt: The vitual address into which to read the xattr data - * @ecryptfs_dentry: The eCryptfs dentry + * @ecryptfs_inode: The eCryptfs inode * * Attempts to read the crypto metadata from the extended attribute * region of the lower file. diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index df70bfa..95be9a9 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -187,11 +187,7 @@ static int ecryptfs_open(struct inode *inode, struct file *file) /* Private value of ecryptfs_dentry allocated in * ecryptfs_lookup() */ struct dentry *lower_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry); - struct inode *lower_inode = NULL; - struct file *lower_file = NULL; - struct vfsmount *lower_mnt; struct ecryptfs_file_info *file_info; - int lower_flags; mount_crypt_stat = ecryptfs_superblock_to_private( ecryptfs_dentry-d_sb)-mount_crypt_stat; @@ -219,26 +215,12 @@ static int ecryptfs_open(struct inode *inode, struct file *file) if (!(crypt_stat-flags ECRYPTFS_POLICY_APPLIED)) { ecryptfs_printk(KERN_DEBUG, Setting flags for stat...\n); /* Policy code enabled in future release */ - crypt_stat-flags |= ECRYPTFS_POLICY_APPLIED; - crypt_stat-flags |= ECRYPTFS_ENCRYPTED; + crypt_stat-flags |= (ECRYPTFS_POLICY_APPLIED + | ECRYPTFS_ENCRYPTED); } mutex_unlock(crypt_stat-cs_mutex); - lower_flags = file-f_flags; - if ((lower_flags O_ACCMODE) == O_WRONLY) - lower_flags = (lower_flags O_ACCMODE) | O_RDWR; - if (file-f_flags O_APPEND) - lower_flags = ~O_APPEND; - lower_mnt = ecryptfs_dentry_to_lower_mnt(ecryptfs_dentry); - /* Corresponding fput() in ecryptfs_release() */ - rc = ecryptfs_open_lower_file(lower_file, lower_dentry, lower_mnt, - lower_flags); - if (rc) { - ecryptfs_printk(KERN_ERR, Error opening lower file\n); - goto out_puts; - } - ecryptfs_set_file_lower(file, lower_file); - /* Isn't this check the same as the one in lookup? */ - lower_inode = lower_dentry-d_inode; + ecryptfs_set_file_lower( + file, ecryptfs_inode_to_private(inode)-lower_file); if (S_ISDIR(ecryptfs_dentry-d_inode-i_mode)) { ecryptfs_printk(KERN_DEBUG, This is a directory\n); crypt_stat-flags = ~(ECRYPTFS_ENCRYPTED); @@ -260,7 +242,7 @@ static int ecryptfs_open(struct inode *inode, struct file *file) and plaintext passthrough mode is not enabled; returning -EIO\n); mutex_unlock(crypt_stat-cs_mutex); - goto out_puts; + goto out_free; } rc = 0; crypt_stat-flags = ~(ECRYPTFS_ENCRYPTED); @@ -272,11 +254,8 @@ static int ecryptfs_open(struct inode *inode, struct file *file) ecryptfs_printk(KERN_DEBUG, inode w/ addr = [0x%p], i_ino = [0x%.16x] size: [0x%.16x]\n, inode, inode-i_ino, i_size_read(inode)); - ecryptfs_set_file_lower(file, lower_file); goto out; -out_puts: - mntput(lower_mnt); - dput(lower_dentry); +out_free: kmem_cache_free(ecryptfs_file_info_cache, ecryptfs_file_to_private(file)); out: @@ -296,20 +275,9 @@ static int ecryptfs_flush(struct file *file, fl_owner_t td) static int ecryptfs_release(struct inode *inode, struct file *file) { - struct file *lower_file = ecryptfs_file_to_lower(file); - struct ecryptfs_file_info *file_info = ecryptfs_file_to_private(file); - struct inode *lower_inode = ecryptfs_inode_to_lower(inode); - int rc; - - rc = ecryptfs_close_lower_file(lower_file); - if (rc) { - printk(KERN_ERR Error closing lower_file\n); - goto out; - } - inode-i_blocks = lower_inode-i_blocks
[PATCH 8/11] eCryptfs: Convert mmap functions to use persistent file
Convert readpage, prepare_write, and commit_write to use read_write.c routines. Remove sync_page; I cannot think of a good reason for implementing that in eCryptfs. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/mmap.c | 199 +++- 1 files changed, 103 insertions(+), 96 deletions(-) diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 60e635e..dd68dd3 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -267,9 +267,78 @@ static void set_header_info(char *page_virt, } /** + * ecryptfs_copy_up_encrypted_with_header + * @page: Sort of a ``virtual'' representation of the encrypted lower + *file. The actual lower file does not have the metadata in + *the header. + * @crypt_stat: The eCryptfs inode's cryptographic context + * + * The ``view'' is the version of the file that userspace winds up + * seeing, with the header information inserted. + */ +static int +ecryptfs_copy_up_encrypted_with_header(struct page *page, + struct ecryptfs_crypt_stat *crypt_stat) +{ + loff_t extent_num_in_page = 0; + loff_t num_extents_per_page = (PAGE_CACHE_SIZE + / crypt_stat-extent_size); + int rc = 0; + + while (extent_num_in_page num_extents_per_page) { + loff_t view_extent_num = ((page-index * num_extents_per_page) + + extent_num_in_page); + + if (view_extent_num crypt_stat-num_header_extents_at_front) { + /* This is a header extent */ + char *page_virt; + + page_virt = kmap_atomic(page, KM_USER0); + memset(page_virt, 0, PAGE_CACHE_SIZE); + /* TODO: Support more than one header extent */ + if (view_extent_num == 0) { + rc = ecryptfs_read_xattr_region( + page_virt, page-mapping-host); + set_header_info(page_virt, crypt_stat); + } + kunmap_atomic(page_virt, KM_USER0); + flush_dcache_page(page); + if (rc) { + ClearPageUptodate(page); + printk(KERN_ERR %s: Error reading xattr + region; rc = [%d]\n, __FUNCTION__, rc); + goto out; + } + SetPageUptodate(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); + + rc = ecryptfs_read_lower_page_segment( + page, (lower_offset PAGE_CACHE_SHIFT), + (lower_offset ~PAGE_CACHE_MASK), + crypt_stat-extent_size, page-mapping-host); + if (rc) { + printk(KERN_ERR %s: Error attempting to read + extent at offset [%lld] in the lower + file; rc = [%d]\n, __FUNCTION__, + lower_offset, rc); + goto out; + } + } + extent_num_in_page++; + } +out: + return rc; +} + +/** * ecryptfs_readpage - * @file: This is an ecryptfs file - * @page: ecryptfs associated page to stick the read data into + * @file: An eCryptfs file + * @page: Page from eCryptfs inode mapping into which to stick the read data * * Read in a page, decrypting if necessary. * @@ -277,59 +346,35 @@ static void set_header_info(char *page_virt, */ static int ecryptfs_readpage(struct file *file, struct page *page) { + struct ecryptfs_crypt_stat *crypt_stat = + ecryptfs_inode_to_private(file-f_path.dentry-d_inode)-crypt_stat; int rc = 0; - struct ecryptfs_crypt_stat *crypt_stat; - BUG_ON(!(file file-f_path.dentry file-f_path.dentry-d_inode)); - crypt_stat = ecryptfs_inode_to_private(file-f_path.dentry-d_inode) - -crypt_stat; if (!crypt_stat || !(crypt_stat-flags ECRYPTFS_ENCRYPTED) || (crypt_stat-flags ECRYPTFS_NEW_FILE)) { ecryptfs_printk(KERN_DEBUG, Passing through unencrypted page\n); - rc = ecryptfs_do_readpage(file, page, page-index); - if (rc) { - ecryptfs_printk(KERN_ERR, Error reading page; rc = - [%d]\n, rc
[PATCH 9/11] eCryptfs: Initialize persistent lower file on inode create
Initialize persistent lower file on inode create. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/super.c | 13 +++-- 1 files changed, 7 insertions(+), 6 deletions(-) diff --git a/fs/ecryptfs/super.c b/fs/ecryptfs/super.c index b97e210..f8cdab2 100644 --- a/fs/ecryptfs/super.c +++ b/fs/ecryptfs/super.c @@ -47,15 +47,16 @@ struct kmem_cache *ecryptfs_inode_info_cache; */ static struct inode *ecryptfs_alloc_inode(struct super_block *sb) { - struct ecryptfs_inode_info *ecryptfs_inode; + struct ecryptfs_inode_info *inode_info; struct inode *inode = NULL; - ecryptfs_inode = kmem_cache_alloc(ecryptfs_inode_info_cache, - GFP_KERNEL); - if (unlikely(!ecryptfs_inode)) + inode_info = kmem_cache_alloc(ecryptfs_inode_info_cache, GFP_KERNEL); + if (unlikely(!inode_info)) goto out; - ecryptfs_init_crypt_stat(ecryptfs_inode-crypt_stat); - inode = ecryptfs_inode-vfs_inode; + ecryptfs_init_crypt_stat(inode_info-crypt_stat); + mutex_init(inode_info-lower_file_mutex); + inode_info-lower_file = NULL; + inode = inode_info-vfs_inode; out: return inode; } -- 1.5.1.6 - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 10/11] eCryptfs: Remove unused functions and kmem_cache
The switch to read_write.c routines and the persistent file make a number of functions unnecessary. This patch removes them. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c | 150 -- fs/ecryptfs/ecryptfs_kernel.h | 21 +--- fs/ecryptfs/file.c| 28 fs/ecryptfs/main.c|5 - fs/ecryptfs/mmap.c| 336 - 5 files changed, 1 insertions(+), 539 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index b3014d7..3b3cf27 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -353,119 +353,6 @@ out: return rc; } -static void -ecryptfs_extent_to_lwr_pg_idx_and_offset(unsigned long *lower_page_idx, -int *byte_offset, -struct ecryptfs_crypt_stat *crypt_stat, -unsigned long extent_num) -{ - unsigned long lower_extent_num; - int extents_occupied_by_headers_at_front; - int bytes_occupied_by_headers_at_front; - int extent_offset; - int extents_per_page; - - bytes_occupied_by_headers_at_front = - (crypt_stat-extent_size -* crypt_stat-num_header_extents_at_front); - extents_occupied_by_headers_at_front = - ( bytes_occupied_by_headers_at_front - / crypt_stat-extent_size ); - lower_extent_num = extents_occupied_by_headers_at_front + extent_num; - extents_per_page = PAGE_CACHE_SIZE / crypt_stat-extent_size; - (*lower_page_idx) = lower_extent_num / extents_per_page; - extent_offset = lower_extent_num % extents_per_page; - (*byte_offset) = extent_offset * crypt_stat-extent_size; - ecryptfs_printk(KERN_DEBUG, * crypt_stat-extent_size = - [%d]\n, crypt_stat-extent_size); - ecryptfs_printk(KERN_DEBUG, * crypt_stat- - num_header_extents_at_front = [%d]\n, - crypt_stat-num_header_extents_at_front); - ecryptfs_printk(KERN_DEBUG, * extents_occupied_by_headers_at_ - front = [%d]\n, extents_occupied_by_headers_at_front); - ecryptfs_printk(KERN_DEBUG, * lower_extent_num = [0x%.16x]\n, - lower_extent_num); - ecryptfs_printk(KERN_DEBUG, * extents_per_page = [%d]\n, - extents_per_page); - ecryptfs_printk(KERN_DEBUG, * (*lower_page_idx) = [0x%.16x]\n, - (*lower_page_idx)); - ecryptfs_printk(KERN_DEBUG, * extent_offset = [%d]\n, - extent_offset); - ecryptfs_printk(KERN_DEBUG, * (*byte_offset) = [%d]\n, - (*byte_offset)); -} - -static int ecryptfs_write_out_page(struct ecryptfs_page_crypt_context *ctx, - struct page *lower_page, - struct inode *lower_inode, - int byte_offset_in_page, int bytes_to_write) -{ - int rc = 0; - - if (ctx-mode == ECRYPTFS_PREPARE_COMMIT_MODE) { - rc = ecryptfs_commit_lower_page(lower_page, lower_inode, - ctx-param.lower_file, - byte_offset_in_page, - bytes_to_write); - if (rc) { - ecryptfs_printk(KERN_ERR, Error calling lower - commit; rc = [%d]\n, rc); - goto out; - } - } else { - rc = ecryptfs_writepage_and_release_lower_page(lower_page, - lower_inode, - ctx-param.wbc); - if (rc) { - ecryptfs_printk(KERN_ERR, Error calling lower - writepage(); rc = [%d]\n, rc); - goto out; - } - } -out: - return rc; -} - -static int ecryptfs_read_in_page(struct ecryptfs_page_crypt_context *ctx, -struct page **lower_page, -struct inode *lower_inode, -unsigned long lower_page_idx, -int byte_offset_in_page) -{ - int rc = 0; - - if (ctx-mode == ECRYPTFS_PREPARE_COMMIT_MODE) { - /* TODO: Limit this to only the data extents that are -* needed */ - rc = ecryptfs_get_lower_page(lower_page, lower_inode, -ctx-param.lower_file, -lower_page_idx, -byte_offset_in_page, -(PAGE_CACHE_SIZE
[PATCH 11/11] eCryptfs: Replace magic numbers
Replace some magic numbers with sizeof() equivalents. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] --- fs/ecryptfs/crypto.c |6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index 3b3cf27..425a144 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c @@ -1426,10 +1426,10 @@ static int parse_header_metadata(struct ecryptfs_crypt_stat *crypt_stat, u32 header_extent_size; u16 num_header_extents_at_front; - memcpy(header_extent_size, virt, 4); + memcpy(header_extent_size, virt, sizeof(u32)); header_extent_size = be32_to_cpu(header_extent_size); - virt += 4; - memcpy(num_header_extents_at_front, virt, 2); + 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; -- 1.5.1.6 - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] eCryptfs: Delay writing 0's after llseek until write
Delay writing 0's out in eCryptfs after a seek past the end of the file until data is actually written. Signed-off-by: Michael Halcrow [EMAIL PROTECTED] diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index 9881b5c..59288d8 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -33,63 +33,6 @@ #include ecryptfs_kernel.h /** - * ecryptfs_llseek - * @file: File we are seeking in - * @offset: The offset to seek to - * @origin: 2 - offset from i_size; 1 - offset from f_pos - * - * Returns the position we have seeked to, or negative on error - */ -static loff_t ecryptfs_llseek(struct file *file, loff_t offset, int origin) -{ - loff_t rv; - loff_t new_end_pos; - int rc; - int expanding_file = 0; - struct inode *inode = file-f_mapping-host; - - /* If our offset is past the end of our file, we're going to -* need to grow it so we have a valid length of 0's */ - new_end_pos = offset; - switch (origin) { - case 2: - new_end_pos += i_size_read(inode); - expanding_file = 1; - break; - case 1: - new_end_pos += file-f_pos; - if (new_end_pos i_size_read(inode)) { - ecryptfs_printk(KERN_DEBUG, new_end_pos(=[0x%.16x]) -i_size_read(inode)(=[0x%.16x])\n, - new_end_pos, i_size_read(inode)); - expanding_file = 1; - } - break; - default: - if (new_end_pos i_size_read(inode)) { - ecryptfs_printk(KERN_DEBUG, new_end_pos(=[0x%.16x]) -i_size_read(inode)(=[0x%.16x])\n, - new_end_pos, i_size_read(inode)); - expanding_file = 1; - } - } - ecryptfs_printk(KERN_DEBUG, new_end_pos = [0x%.16x]\n, new_end_pos); - if (expanding_file) { - rc = ecryptfs_truncate(file-f_path.dentry, new_end_pos); - if (rc) { - rv = rc; - ecryptfs_printk(KERN_ERR, Error on attempt to - truncate to (higher) offset [0x%.16x]; -rc = [%d]\n, new_end_pos, rc); - goto out; - } - } - rv = generic_file_llseek(file, offset, origin); -out: - return rv; -} - -/** * ecryptfs_read_update_atime * * generic_file_read updates the atime of upper layer inode. But, it @@ -425,7 +368,7 @@ const struct file_operations ecryptfs_dir_fops = { }; const struct file_operations ecryptfs_main_fops = { - .llseek = ecryptfs_llseek, + .llseek = generic_file_llseek, .read = do_sync_read, .aio_read = ecryptfs_read_update_atime, .write = do_sync_write, diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 88ea669..55cec98 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -376,9 +376,31 @@ out: return 0; } +/** + * eCryptfs does not currently support holes. When writing after a + * seek past the end of the file, eCryptfs fills in 0's through to the + * current location. The code to fill in the 0's to all the + * intermediate pages calls ecryptfs_prepare_write_no_truncate(). + */ +static int +ecryptfs_prepare_write_no_truncate(struct file *file, struct page *page, + unsigned from, unsigned to) +{ + int rc = 0; + + if (from == 0 to == PAGE_CACHE_SIZE) + goto out; /* If we are writing a full page, it will be + up to date. */ + if (!PageUptodate(page)) + rc = ecryptfs_do_readpage(file, page, page-index); +out: + return rc; +} + static int ecryptfs_prepare_write(struct file *file, struct page *page, unsigned from, unsigned to) { + loff_t pos; int rc = 0; if (from == 0 to == PAGE_CACHE_SIZE) @@ -386,6 +408,16 @@ static int ecryptfs_prepare_write(struct file *file, struct page *page, up to date. */ if (!PageUptodate(page)) rc = ecryptfs_do_readpage(file, page, page-index); + pos = ((loff_t)page-index PAGE_CACHE_SHIFT) + to; + if (pos i_size_read(page-mapping-host)) { + rc = ecryptfs_truncate(file-f_path.dentry, pos); + if (rc) { + printk(KERN_ERR Error on attempt to + truncate to (higher) offset [%lld]; + rc = [%d]\n, pos, rc); + goto out; + } + } out: return rc; } @@ -744,10 +776,10 @@ int write_zeros(struct file *file, pgoff_t index, int start, int num_zeros) rc = PTR_ERR(tmp_page); goto out
Re: [PATCH 01/24] Unionfs: Documentation
On Mon, Jan 08, 2007 at 03:51:31PM -0500, Erez Zadok wrote: BTW, this is a problem with all stackable file systems, including ecryptfs. To be fair, our Unionfs users have come up against this problem, usually for the first time they use Unionfs :-). I suspect that the only reason why this has not yet surfaced as a major issue in eCryptfs is because nobody is bothering to manipulate the eCryptfs-encrypted lower files. The only code out there right now that can make sense of the files is in the eCryptfs kernel module. Now, we've discussed a number of possible solutions. Thanks to suggestions we got at OLS, we discussed a way to hide the lower namespace, or make it readonly, using existing kernel facilities. But my understanding is that even it'd work, it'd only address new processes: if an existing process has an open fd in a lower branch before we lock up the lower branch's name space, that process may still be able to make lower-level changes. Again, eCryptfs is fortunate in that the vast majority of users who access the lower eCryptfs files will only want to read the encrypted files (to do backups, for instance). I do not know of any userspace utilities that can write correct eCryptfs lower file content. Detecting such processes may not be easy. What to do with them, once detected, is also unclear. We welcome suggestions. My first instinct is to say that stacked filesystem should not even begin to open the file if it is already opened by something other than the stacked filesystem (-EPERM with a message in the syslog about the problem). In the case when a stacked filesystem wants to open a file that is already opened by something other than the stacked filesystem, the stacked filesystem loses. Once the process closes the file, the process is hitherto prevented from accessing the file again (via the before-mentioned mechanism of hiding the lower namespace). Another possibility is that after, hopefully, both Unionfs and ecryptfs are in, and some more user experience has been accumulated, that we'll look into addressing this page-cache consistency problem for all stacked f/s. Unionfs and eCryptfs share almost exactly the same namespace issues. Unionfs happens to be impacted by them more than eCryptfs because of the differences in how people actually access the files under the two filesystems. Jeff, I don't think it's acceptable to OOPS. For now, stacked filesystems just need to stay on their toes. There are several places where assumptions need to be checked. Mike - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html