From ae73fc093a8cae4d92e22ab8b635e3590e80785d Mon Sep 17 00:00:00 2001 From: Michael Halcrow Date: Wed, 28 Feb 2007 20:12:16 -0800 Subject: [PATCH] eCryptfs: resolve lower page unlocking problem eCryptfs lower file handling code has several issues: - Retval from prepare_write()/commit_write() wasn't checked to equality to AOP_TRUNCATED_PAGE. - In some places page wasn't unmapped and unlocked after error. Signed-off-by: Michael Halcrow Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/mmap.c | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 3a6f65c3f14f..82bdbed658d7 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -422,9 +422,11 @@ out: return rc; } -static void ecryptfs_release_lower_page(struct page *lower_page) +static +void ecryptfs_release_lower_page(struct page *lower_page, int page_locked) { - unlock_page(lower_page); + if (page_locked) + unlock_page(lower_page); page_cache_release(lower_page); } @@ -454,6 +456,13 @@ static int ecryptfs_write_inode_size_to_header(struct file *lower_file, } lower_a_ops = lower_inode->i_mapping->a_ops; rc = lower_a_ops->prepare_write(lower_file, header_page, 0, 8); + if (rc) { + if (rc == AOP_TRUNCATED_PAGE) + ecryptfs_release_lower_page(header_page, 0); + else + ecryptfs_release_lower_page(header_page, 1); + goto out; + } file_size = (u64)i_size_read(inode); ecryptfs_printk(KERN_DEBUG, "Writing size: [0x%.16x]\n", file_size); file_size = cpu_to_be64(file_size); @@ -465,7 +474,10 @@ static int ecryptfs_write_inode_size_to_header(struct file *lower_file, if (rc < 0) ecryptfs_printk(KERN_ERR, "Error commiting header page " "write\n"); - ecryptfs_release_lower_page(header_page); + if (rc == AOP_TRUNCATED_PAGE) + ecryptfs_release_lower_page(header_page, 0); + else + ecryptfs_release_lower_page(header_page, 1); lower_inode->i_mtime = lower_inode->i_ctime = CURRENT_TIME; mark_inode_dirty_sync(inode); out: @@ -572,7 +584,10 @@ int ecryptfs_get_lower_page(struct page **lower_page, struct inode *lower_inode, } out: if (rc && (*lower_page)) { - ecryptfs_release_lower_page(*lower_page); + if (rc == AOP_TRUNCATED_PAGE) + ecryptfs_release_lower_page(*lower_page, 0); + else + ecryptfs_release_lower_page(*lower_page, 1); (*lower_page) = NULL; } return rc; @@ -588,16 +603,19 @@ ecryptfs_commit_lower_page(struct page *lower_page, struct inode *lower_inode, struct file *lower_file, int byte_offset, int region_size) { + int page_locked = 1; int rc = 0; rc = lower_inode->i_mapping->a_ops->commit_write( lower_file, lower_page, byte_offset, region_size); + if (rc == AOP_TRUNCATED_PAGE) + page_locked = 0; if (rc < 0) { ecryptfs_printk(KERN_ERR, "Error committing write; rc = [%d]\n", rc); } else rc = 0; - ecryptfs_release_lower_page(lower_page); + ecryptfs_release_lower_page(lower_page, page_locked); return rc; } -- cgit v1.2.3 From a8d547d5cf3df447d1527f2e66ba578e88011999 Mon Sep 17 00:00:00 2001 From: Michael Halcrow Date: Wed, 28 Feb 2007 20:12:47 -0800 Subject: [PATCH] eCryptfs: set O_LARGEFILE when opening lower file O_LARGEFILE should be set here when opening the lower file. Signed-off-by: Michael Halcrow Cc: Dmitriy Monakhov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/file.c | 1 + fs/ecryptfs/inode.c | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index bd969adf70d7..7a7d25d541e7 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c @@ -205,6 +205,7 @@ int ecryptfs_open_lower_file(struct file **lower_file, { int rc = 0; + flags |= O_LARGEFILE; dget(lower_dentry); mntget(lower_mnt); *lower_file = dentry_open(lower_dentry, lower_mnt, flags); diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 9fa7e0b27a96..0cfff4fefa9e 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -200,9 +200,6 @@ static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry) inode = ecryptfs_dentry->d_inode; crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat; lower_flags = ((O_CREAT | O_TRUNC) & O_ACCMODE) | O_RDWR; -#if BITS_PER_LONG != 32 - lower_flags |= O_LARGEFILE; -#endif 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, -- cgit v1.2.3 From 1ed6d896de3a57bdfb38cffaa748612f112c2a75 Mon Sep 17 00:00:00 2001 From: Michael Halcrow Date: Wed, 28 Feb 2007 20:12:52 -0800 Subject: [PATCH] eCryptfs: remove unnecessary flush_dcache_page() Remove unnecessary flush_dcache_page() call. Thanks to Dmitriy Monakhov for pointing this out. Signed-off-by: Michael Halcrow Cc: Dmitriy Monakhov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/mmap.c | 1 - 1 file changed, 1 deletion(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 82bdbed658d7..7be8e91b5ba0 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -238,7 +238,6 @@ int ecryptfs_do_readpage(struct file *file, struct page *page, lower_page_data = kmap_atomic(lower_page, KM_USER1); memcpy(page_data, lower_page_data, PAGE_CACHE_SIZE); kunmap_atomic(lower_page_data, KM_USER1); - flush_dcache_page(lower_page); kunmap_atomic(page_data, KM_USER0); flush_dcache_page(page); rc = 0; -- cgit v1.2.3 From 65dc8145711d1c20aecbb1a8a4a518f7c68611b8 Mon Sep 17 00:00:00 2001 From: Michael Halcrow Date: Wed, 28 Feb 2007 20:12:57 -0800 Subject: [PATCH] eCryptfs: no path_release() after path_lookup() error Dmitriy Monakhov wrote: > if path_lookup() return non zero code we don't have to worry about > 'nd' parameter, but ecryptfs_read_super does path_release(&nd) after > path_lookup has failed, and dentry counter becomes negative Do not do a path_release after a path_lookup error. Signed-off-by: Michael Halcrow Cc: Dmitriy Monakhov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 80044d196fe0..812427e6805c 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -487,7 +487,7 @@ static int ecryptfs_read_super(struct super_block *sb, const char *dev_name) rc = path_lookup(dev_name, LOOKUP_FOLLOW, &nd); if (rc) { ecryptfs_printk(KERN_WARNING, "path_lookup() failed\n"); - goto out_free; + goto out; } lower_root = nd.dentry; if (!lower_root->d_inode) { -- cgit v1.2.3 From ad5f1196792653dadf09c07a5fa917092b469c1c Mon Sep 17 00:00:00 2001 From: Dmitriy Monakhov Date: Mon, 5 Mar 2007 00:30:12 -0800 Subject: [PATCH] ecryptfs: check xattr operation support fix - ecryptfs_write_inode_size_to_metadata() error code was ignored. - i_op->setxattr() must be supported by lower fs because used below. Signed-off-by: Monakhov Dmitriy Acked-by: Michael Halcrow Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/inode.c | 6 +++--- fs/ecryptfs/mmap.c | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index 0cfff4fefa9e..e62f3fc7241e 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -168,9 +168,9 @@ static int grow_file(struct dentry *ecryptfs_dentry, struct file *lower_file, goto out; } i_size_write(inode, 0); - ecryptfs_write_inode_size_to_metadata(lower_file, lower_inode, inode, - ecryptfs_dentry, - ECRYPTFS_LOWER_I_MUTEX_NOT_HELD); + rc = ecryptfs_write_inode_size_to_metadata(lower_file, lower_inode, + inode, ecryptfs_dentry, + ECRYPTFS_LOWER_I_MUTEX_NOT_HELD); ecryptfs_inode_to_private(inode)->crypt_stat.flags |= ECRYPTFS_NEW_FILE; out: return rc; diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 7be8e91b5ba0..7def4be83e61 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -502,7 +502,8 @@ static int ecryptfs_write_inode_size_to_xattr(struct inode *lower_inode, goto out; } lower_dentry = ecryptfs_dentry_to_lower(ecryptfs_dentry); - if (!lower_dentry->d_inode->i_op->getxattr) { + if (!lower_dentry->d_inode->i_op->getxattr || + !lower_dentry->d_inode->i_op->setxattr) { printk(KERN_WARNING "No support for setting xattr in lower filesystem\n"); rc = -ENOSYS; -- cgit v1.2.3 From 82b16528405131eadc18285da982d4806f6db34e Mon Sep 17 00:00:00 2001 From: Dmitriy Monakhov Date: Mon, 5 Mar 2007 00:30:46 -0800 Subject: [PATCH] ecryptfs: lower root result must be adirectory - Currently after path_lookup succeed we dot't have any guarantie what it is DIR. This must be explicitly demanded. - path_lookup can't return negative dentry, So inode check is useless. Signed-off-by: Dmitriy Monakhov Acked-by: Michael Halcrow Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/main.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 812427e6805c..fc4a3a224641 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c @@ -484,18 +484,12 @@ static int ecryptfs_read_super(struct super_block *sb, const char *dev_name) struct vfsmount *lower_mnt; memset(&nd, 0, sizeof(struct nameidata)); - rc = path_lookup(dev_name, LOOKUP_FOLLOW, &nd); + rc = path_lookup(dev_name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &nd); if (rc) { ecryptfs_printk(KERN_WARNING, "path_lookup() failed\n"); goto out; } lower_root = nd.dentry; - if (!lower_root->d_inode) { - ecryptfs_printk(KERN_WARNING, - "No directory to interpose on\n"); - rc = -ENOENT; - goto out_free; - } lower_mnt = nd.mnt; ecryptfs_set_superblock_lower(sb, lower_root->d_sb); sb->s_maxbytes = lower_root->d_sb->s_maxbytes; -- cgit v1.2.3 From a8fa74ab529f23f812092ece8d7a4766af092332 Mon Sep 17 00:00:00 2001 From: Dmitriy Monakhov Date: Mon, 5 Mar 2007 00:30:47 -0800 Subject: [PATCH] ecryptfs: handle AOP_TRUNCATED_PAGE better - In fact we don't have to fail if AOP_TRUNCATED_PAGE was returned from prepare_write or commit_write. It is beter to retry attempt where it is possible. - Rearange ecryptfs_get_lower_page() error handling logic, make it more clean. Signed-off-by: Dmitriy Monakhov Acked-by: Michael Halcrow Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/mmap.c | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/mmap.c b/fs/ecryptfs/mmap.c index 7def4be83e61..b731b09499cb 100644 --- a/fs/ecryptfs/mmap.c +++ b/fs/ecryptfs/mmap.c @@ -446,6 +446,7 @@ static int ecryptfs_write_inode_size_to_header(struct file *lower_file, const struct address_space_operations *lower_a_ops; u64 file_size; +retry: header_page = grab_cache_page(lower_inode->i_mapping, 0); if (!header_page) { ecryptfs_printk(KERN_ERR, "grab_cache_page for " @@ -456,9 +457,10 @@ static int ecryptfs_write_inode_size_to_header(struct file *lower_file, lower_a_ops = lower_inode->i_mapping->a_ops; rc = lower_a_ops->prepare_write(lower_file, header_page, 0, 8); if (rc) { - if (rc == AOP_TRUNCATED_PAGE) + if (rc == AOP_TRUNCATED_PAGE) { ecryptfs_release_lower_page(header_page, 0); - else + goto retry; + } else ecryptfs_release_lower_page(header_page, 1); goto out; } @@ -473,9 +475,10 @@ static int ecryptfs_write_inode_size_to_header(struct file *lower_file, if (rc < 0) ecryptfs_printk(KERN_ERR, "Error commiting header page " "write\n"); - if (rc == AOP_TRUNCATED_PAGE) + if (rc == AOP_TRUNCATED_PAGE) { ecryptfs_release_lower_page(header_page, 0); - else + goto retry; + } else ecryptfs_release_lower_page(header_page, 1); lower_inode->i_mtime = lower_inode->i_ctime = CURRENT_TIME; mark_inode_dirty_sync(inode); @@ -565,6 +568,7 @@ int ecryptfs_get_lower_page(struct page **lower_page, struct inode *lower_inode, { int rc = 0; +retry: *lower_page = grab_cache_page(lower_inode->i_mapping, lower_page_index); if (!(*lower_page)) { rc = -EINVAL; @@ -578,18 +582,18 @@ int ecryptfs_get_lower_page(struct page **lower_page, struct inode *lower_inode, byte_offset, region_bytes); if (rc) { - ecryptfs_printk(KERN_ERR, "prepare_write for " + if (rc == AOP_TRUNCATED_PAGE) { + ecryptfs_release_lower_page(*lower_page, 0); + goto retry; + } else { + ecryptfs_printk(KERN_ERR, "prepare_write for " "lower_page_index = [0x%.16x] failed; rc = " "[%d]\n", lower_page_index, rc); - } -out: - if (rc && (*lower_page)) { - if (rc == AOP_TRUNCATED_PAGE) - ecryptfs_release_lower_page(*lower_page, 0); - else ecryptfs_release_lower_page(*lower_page, 1); - (*lower_page) = NULL; + (*lower_page) = NULL; + } } +out: return rc; } -- cgit v1.2.3 From 908e0a8a265fe8057604a9a30aec3f0be7bb5ebb Mon Sep 17 00:00:00 2001 From: Peter Zijlstra Date: Wed, 7 Mar 2007 20:41:30 -0800 Subject: [PATCH] ecryptfs: nested locking annotation ecryptfs uses a lock_parent() function, which I hope really locks the parents and is not abused Signed-off-by: Peter Zijlstra Cc: Michael Halcrow Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index e62f3fc7241e..1548be26b5e6 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c @@ -38,7 +38,7 @@ static struct dentry *lock_parent(struct dentry *dentry) struct dentry *dir; dir = dget(dentry->d_parent); - mutex_lock(&(dir->d_inode->i_mutex)); + mutex_lock_nested(&(dir->d_inode->i_mutex), I_MUTEX_PARENT); return dir; } -- cgit v1.2.3 From b228b8e5bf96b740a70871c1a248bb65c267f5f2 Mon Sep 17 00:00:00 2001 From: Michael Halcrow Date: Fri, 16 Mar 2007 13:38:22 -0800 Subject: [PATCH] eCryptfs: fix possible NULL ptr deref in ecryptfs_d_release() ecryptfs_d_release() first dereferences a pointer (via ecryptfs_dentry_to_lower()) and then afterwards checks to see if the pointer it just dereferenced is NULL (via ecryptfs_dentry_to_private()). This patch moves all of the work done on the dereferenced pointer inside a block governed by the condition that the pointer is non-NULL. Signed-off-by: Michael Halcrow Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ecryptfs/dentry.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/dentry.c b/fs/ecryptfs/dentry.c index 329efcd3d8c9..cb20b964419f 100644 --- a/fs/ecryptfs/dentry.c +++ b/fs/ecryptfs/dentry.c @@ -78,18 +78,13 @@ struct kmem_cache *ecryptfs_dentry_info_cache; */ static void ecryptfs_d_release(struct dentry *dentry) { - struct dentry *lower_dentry; - - lower_dentry = ecryptfs_dentry_to_lower(dentry); - if (ecryptfs_dentry_to_private(dentry)) + if (ecryptfs_dentry_to_private(dentry)) { + if (ecryptfs_dentry_to_lower(dentry)) { + mntput(ecryptfs_dentry_to_lower_mnt(dentry)); + dput(ecryptfs_dentry_to_lower(dentry)); + } kmem_cache_free(ecryptfs_dentry_info_cache, ecryptfs_dentry_to_private(dentry)); - if (lower_dentry) { - struct vfsmount *lower_mnt = - ecryptfs_dentry_to_lower_mnt(dentry); - - mntput(lower_mnt); - dput(lower_dentry); } return; } -- cgit v1.2.3 From b529ccf2799c14346d1518e9bdf1f88f03643e99 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 25 Apr 2007 19:08:35 -0700 Subject: [NETLINK]: Introduce nlmsg_hdr() helper For the common "(struct nlmsghdr *)skb->data" sequence, so that we reduce the number of direct accesses to skb->data and for consistency with all the other cast skb member helpers. Signed-off-by: Arnaldo Carvalho de Melo Signed-off-by: David S. Miller --- fs/ecryptfs/netlink.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/netlink.c b/fs/ecryptfs/netlink.c index e3aa2253c850..8405d216a5fc 100644 --- a/fs/ecryptfs/netlink.c +++ b/fs/ecryptfs/netlink.c @@ -97,7 +97,7 @@ out: */ static int ecryptfs_process_nl_response(struct sk_buff *skb) { - struct nlmsghdr *nlh = (struct nlmsghdr*)skb->data; + struct nlmsghdr *nlh = nlmsg_hdr(skb); struct ecryptfs_message *msg = NLMSG_DATA(nlh); int rc; @@ -181,7 +181,7 @@ receive: "rc = [%d]\n", rc); return; } - nlh = (struct nlmsghdr *)skb->data; + nlh = nlmsg_hdr(skb); if (!NLMSG_OK(nlh, skb->len)) { ecryptfs_printk(KERN_ERR, "Received corrupt netlink " "message\n"); -- cgit v1.2.3 From af65bdfce98d7965fbe93a48b8128444a2eea024 Mon Sep 17 00:00:00 2001 From: Patrick McHardy Date: Fri, 20 Apr 2007 14:14:21 -0700 Subject: [NETLINK]: Switch cb_lock spinlock to mutex and allow to override it Switch cb_lock to mutex and allow netlink kernel users to override it with a subsystem specific mutex for consistent locking in dump callbacks. All netlink_dump_start users have been audited not to rely on any side-effects of the previously used spinlock. Signed-off-by: Patrick McHardy Signed-off-by: David S. Miller --- fs/ecryptfs/netlink.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/ecryptfs') diff --git a/fs/ecryptfs/netlink.c b/fs/ecryptfs/netlink.c index 8405d216a5fc..fe9186312d7c 100644 --- a/fs/ecryptfs/netlink.c +++ b/fs/ecryptfs/netlink.c @@ -229,7 +229,7 @@ int ecryptfs_init_netlink(void) ecryptfs_nl_sock = netlink_kernel_create(NETLINK_ECRYPTFS, 0, ecryptfs_receive_nl_message, - THIS_MODULE); + NULL, THIS_MODULE); if (!ecryptfs_nl_sock) { rc = -EIO; ecryptfs_printk(KERN_ERR, "Failed to create netlink socket\n"); -- cgit v1.2.3