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); + unlock_page(ecryptfs_page); page_cache_release(ecryptfs_page); goto out; } @@ -182,9 +193,11 @@ int ecryptfs_write(struct file *ecryptfs_file, char *data, loff_t offset, if (rc) { printk(KERN_ERR "%s: Error encrypting " "page; rc = [%d]\n", __FUNCTION__, rc); + unlock_page(ecryptfs_page); page_cache_release(ecryptfs_page); goto out; } + unlock_page(ecryptfs_page); page_cache_release(ecryptfs_page); pos += num_bytes; } @@ -245,10 +258,6 @@ int ecryptfs_read_lower(char *data, loff_t offset, size_t size, data_page = virt_to_page(data + i); flush_dcache_page(data_page); - if (rc) - ClearPageUptodate(data_page); - else - SetPageUptodate(data_page); } return rc; } @@ -273,14 +282,34 @@ int ecryptfs_read_lower_page_segment(struct page *page_for_ecryptfs, size_t offset_in_page, size_t size, struct inode *ecryptfs_inode) { - char *virt; + char *page_for_ecryptfs_virt; + char *tmp_virt; loff_t offset; int rc; - offset = ((page_index << PAGE_CACHE_SHIFT) + offset_in_page); - virt = kmap(page_for_ecryptfs); - rc = ecryptfs_read_lower(virt, offset, size, ecryptfs_inode); - kunmap(page_for_ecryptfs); + tmp_virt = kmalloc(PAGE_CACHE_SIZE, GFP_KERNEL); + if (!tmp_virt) { + rc = -ENOMEM; + goto out; + } + offset = ((((loff_t)page_index) << PAGE_CACHE_SHIFT) + offset_in_page); + rc = ecryptfs_read_lower(tmp_virt, offset, size, ecryptfs_inode); + if (rc) { + printk(KERN_ERR "%s: Error reading lower; rc = [%d]\n", + __FUNCTION__, rc); + goto out_free; + } + page_for_ecryptfs_virt = kmap_atomic(page_for_ecryptfs, KM_USER0); + memcpy(page_for_ecryptfs_virt, tmp_virt, PAGE_CACHE_SIZE); + kunmap_atomic(page_for_ecryptfs_virt, KM_USER0); + flush_dcache_page(page_for_ecryptfs); +out_free: + kfree(tmp_virt); +out: + if (rc) + ClearPageUptodate(page_for_ecryptfs); + else + SetPageUptodate(page_for_ecryptfs); return rc; } @@ -328,8 +357,8 @@ int ecryptfs_read(char *data, loff_t offset, size_t size, if (num_bytes > total_remaining_bytes) num_bytes = total_remaining_bytes; - 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 " @@ -342,6 +371,7 @@ int ecryptfs_read(char *data, loff_t offset, size_t size, if (rc) { printk(KERN_ERR "%s: Error decrypting " "page; rc = [%d]\n", __FUNCTION__, rc); + unlock_page(ecryptfs_page); page_cache_release(ecryptfs_page); goto out; } @@ -350,6 +380,7 @@ int ecryptfs_read(char *data, loff_t offset, size_t size, ((char *)ecryptfs_page_virt + start_offset_in_page), num_bytes); kunmap_atomic(ecryptfs_page_virt, KM_USER0); + unlock_page(ecryptfs_page); page_cache_release(ecryptfs_page); pos += num_bytes; data_offset += num_bytes; - 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
