On 2012-03-13 19:39:49, Li Wang wrote: > eCryptfs did not handle the writing for mmaped non-eCryptfs file. > Instead, it put BUG_ON(!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) > on ecryptfs_writepage call path. This patch enables eCryptfs to > deal with such case, to fully support non-eCryptfs operations as it > claims. Hi Li - Thanks for the patch! Before I review/merge it, I'm curious if you use the ecryptfs_plaintext_passthrough feature? IMO, it is a terrible design and I've found that hardly anyone uses it. Maybe *no one*, after knowing that mmap was broken for plaintext files... Having to pre-create the file in the lower filesystem in order for it to be a "plaintext" file inside the eCryptfs mount is a terrible idea. The admin/user/application has no idea if the file is plaintext by looking at it inside the eCryptfs mount and he/she has to examine the file in the lower filesystem to find out. If that file is ever unlinked and then created again, it turns into an encrypted file. There is too much uncertainty around this feature for it to exist in a cryptographic filesystem. I'd really like to continue focusing on getting eCryptfs as stable as possible and then dump the CONFIG_EXPERIMENTAL dependency at some point. As a prerequisite to that, I'm thinking that ecryptfs_plaintext_passthrough support should be removed, at least in its current form. Any thoughts? Tyler > > --- > > To make the bug present > > cd cipher // enter eCryptfs cipher text folder > echo "123" > foo // make non-eCryptfs file > cd .. > mount -t ecryptfs cipher plain -o ecryptfs_passthrough // allow for non-eCryptfs files to be read and written from within an eCryptfs mount > cd plain > run the following program > > int main() > { > int fd = open("foo", O_RDWR); > char * addr; > addr = (char *)mmap(NULL, 256, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); > add[0] = '3'; > munmap(addr, 256); > close(fd); > return 0; > } > > Signed-off-by: Li Wang <liwang@xxxxxxxxxxx> > Signed-off-by: Yunchuan Wen <wenyunchuan@xxxxxxxxxxxxxx> > --- > fs/ecryptfs/mmap.c | 26 ++++++++++++++++++++++++++ > 1 files changed, 26 insertions(+), 0 deletions(-) > > diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c > index 10ec695..a4be0e9 100644 > --- a/fs/ecryptfs/mmap.c > +++ b/fs/ecryptfs/mmap.c > @@ -65,6 +65,32 @@ struct page *ecryptfs_get_locked_page(struct inode *inode, loff_t index) > static int ecryptfs_writepage(struct page *page, struct writeback_control *wbc) > { > int rc; > + struct inode *inode; > + struct ecryptfs_crypt_stat *crypt_stat; > + > + inode = page->mapping->host; > + crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat; > + if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) { > + size_t size; > + loff_t file_size = i_size_read(inode); > + pgoff_t end_page_index = file_size >> PAGE_CACHE_SHIFT; > + > + if (end_page_index < page->index) > + size = 0; > + else if (end_page_index == page->index) > + size = file_size & ~PAGE_CACHE_MASK; > + else > + size = PAGE_CACHE_SIZE; > + > + rc = ecryptfs_write_lower_page_segment(inode, page, 0, size); > + if (unlikely(rc)) { > + ecryptfs_printk(KERN_WARNING, "Error write " > + "page (upper index [0x%.16lx])\n", page->index); > + ClearPageUptodate(page); > + } else > + SetPageUptodate(page); > + goto out; > + } > > /* > * Refuse to write the page out if we are called from reclaim context > -- > 1.7.6.5 >
Attachment:
signature.asc
Description: Digital signature