diff options
| author | Tyler Hicks <tyhicks@linux.vnet.ibm.com> | 2011-05-24 04:49:02 -0400 |
|---|---|---|
| committer | Tyler Hicks <tyhicks@linux.vnet.ibm.com> | 2011-05-29 15:23:39 -0400 |
| commit | 3b06b3ebf44170c90c893c6c80916db6e922b9f2 (patch) | |
| tree | 1da70b311ad7ec7615f3d44ae269ca56a458fc2d | |
| parent | 5ccf92037c7c6e6f28175fd245284923f939259f (diff) | |
eCryptfs: Fix new inode race condition
Only unlock and d_add() new inodes after the plaintext inode size has
been read from the lower filesystem. This fixes a race condition that
was sometimes seen during a multi-job kernel build in an eCryptfs mount.
https://bugzilla.kernel.org/show_bug.cgi?id=36002
Signed-off-by: Tyler Hicks <tyhicks@linux.vnet.ibm.com>
Reported-by: David <david@unsolicited.net>
Tested-by: David <david@unsolicited.net>
| -rw-r--r-- | fs/ecryptfs/crypto.c | 4 | ||||
| -rw-r--r-- | fs/ecryptfs/ecryptfs_kernel.h | 4 | ||||
| -rw-r--r-- | fs/ecryptfs/file.c | 2 | ||||
| -rw-r--r-- | fs/ecryptfs/inode.c | 42 | ||||
| -rw-r--r-- | fs/ecryptfs/main.c | 6 |
5 files changed, 30 insertions, 28 deletions
diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c index b8d5c8091024..f48c4987a15c 100644 --- a/fs/ecryptfs/crypto.c +++ b/fs/ecryptfs/crypto.c | |||
| @@ -1568,11 +1568,11 @@ out: | |||
| 1568 | } | 1568 | } |
| 1569 | 1569 | ||
| 1570 | int ecryptfs_read_and_validate_xattr_region(char *page_virt, | 1570 | int ecryptfs_read_and_validate_xattr_region(char *page_virt, |
| 1571 | struct dentry *ecryptfs_dentry) | 1571 | struct inode *inode) |
| 1572 | { | 1572 | { |
| 1573 | int rc; | 1573 | int rc; |
| 1574 | 1574 | ||
| 1575 | rc = ecryptfs_read_xattr_region(page_virt, ecryptfs_dentry->d_inode); | 1575 | rc = ecryptfs_read_xattr_region(page_virt, inode); |
| 1576 | if (rc) | 1576 | if (rc) |
| 1577 | goto out; | 1577 | goto out; |
| 1578 | if (!contains_ecryptfs_marker(page_virt + ECRYPTFS_FILE_SIZE_BYTES)) { | 1578 | if (!contains_ecryptfs_marker(page_virt + ECRYPTFS_FILE_SIZE_BYTES)) { |
diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h index 41a453236371..72aa24a4c71e 100644 --- a/fs/ecryptfs/ecryptfs_kernel.h +++ b/fs/ecryptfs/ecryptfs_kernel.h | |||
| @@ -662,7 +662,7 @@ void ecryptfs_write_crypt_stat_flags(char *page_virt, | |||
| 662 | int ecryptfs_read_and_validate_header_region(char *data, | 662 | int ecryptfs_read_and_validate_header_region(char *data, |
| 663 | struct inode *ecryptfs_inode); | 663 | struct inode *ecryptfs_inode); |
| 664 | int ecryptfs_read_and_validate_xattr_region(char *page_virt, | 664 | int ecryptfs_read_and_validate_xattr_region(char *page_virt, |
| 665 | struct dentry *ecryptfs_dentry); | 665 | struct inode *inode); |
| 666 | u8 ecryptfs_code_for_cipher_string(char *cipher_name, size_t key_bytes); | 666 | u8 ecryptfs_code_for_cipher_string(char *cipher_name, size_t key_bytes); |
| 667 | int ecryptfs_cipher_code_to_string(char *str, u8 cipher_code); | 667 | int ecryptfs_cipher_code_to_string(char *str, u8 cipher_code); |
| 668 | void ecryptfs_set_default_sizes(struct ecryptfs_crypt_stat *crypt_stat); | 668 | void ecryptfs_set_default_sizes(struct ecryptfs_crypt_stat *crypt_stat); |
| @@ -753,7 +753,7 @@ int ecryptfs_privileged_open(struct file **lower_file, | |||
| 753 | struct dentry *lower_dentry, | 753 | struct dentry *lower_dentry, |
| 754 | struct vfsmount *lower_mnt, | 754 | struct vfsmount *lower_mnt, |
| 755 | const struct cred *cred); | 755 | const struct cred *cred); |
| 756 | int ecryptfs_get_lower_file(struct dentry *ecryptfs_dentry); | 756 | int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode); |
| 757 | void ecryptfs_put_lower_file(struct inode *inode); | 757 | void ecryptfs_put_lower_file(struct inode *inode); |
| 758 | int | 758 | int |
| 759 | ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes, | 759 | ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes, |
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c index 566e5472f78c..4ec9eb00a241 100644 --- a/fs/ecryptfs/file.c +++ b/fs/ecryptfs/file.c | |||
| @@ -191,7 +191,7 @@ static int ecryptfs_open(struct inode *inode, struct file *file) | |||
| 191 | | ECRYPTFS_ENCRYPTED); | 191 | | ECRYPTFS_ENCRYPTED); |
| 192 | } | 192 | } |
| 193 | mutex_unlock(&crypt_stat->cs_mutex); | 193 | mutex_unlock(&crypt_stat->cs_mutex); |
| 194 | rc = ecryptfs_get_lower_file(ecryptfs_dentry); | 194 | rc = ecryptfs_get_lower_file(ecryptfs_dentry, inode); |
| 195 | if (rc) { | 195 | if (rc) { |
| 196 | printk(KERN_ERR "%s: Error attempting to initialize " | 196 | printk(KERN_ERR "%s: Error attempting to initialize " |
| 197 | "the lower file for the dentry with name " | 197 | "the lower file for the dentry with name " |
diff --git a/fs/ecryptfs/inode.c b/fs/ecryptfs/inode.c index fc7d2b748503..f0ad965d7d51 100644 --- a/fs/ecryptfs/inode.c +++ b/fs/ecryptfs/inode.c | |||
| @@ -259,7 +259,8 @@ static int ecryptfs_initialize_file(struct dentry *ecryptfs_dentry) | |||
| 259 | "context; rc = [%d]\n", rc); | 259 | "context; rc = [%d]\n", rc); |
| 260 | goto out; | 260 | goto out; |
| 261 | } | 261 | } |
| 262 | rc = ecryptfs_get_lower_file(ecryptfs_dentry); | 262 | rc = ecryptfs_get_lower_file(ecryptfs_dentry, |
| 263 | ecryptfs_dentry->d_inode); | ||
| 263 | if (rc) { | 264 | if (rc) { |
| 264 | printk(KERN_ERR "%s: Error attempting to initialize " | 265 | printk(KERN_ERR "%s: Error attempting to initialize " |
| 265 | "the lower file for the dentry with name " | 266 | "the lower file for the dentry with name " |
| @@ -350,50 +351,51 @@ static int ecryptfs_lookup_interpose(struct dentry *ecryptfs_dentry, | |||
| 350 | __func__, rc); | 351 | __func__, rc); |
| 351 | goto out; | 352 | goto out; |
| 352 | } | 353 | } |
| 353 | if (inode->i_state & I_NEW) | 354 | if (!S_ISREG(inode->i_mode)) { |
| 354 | unlock_new_inode(inode); | 355 | if (inode->i_state & I_NEW) |
| 355 | d_add(ecryptfs_dentry, inode); | 356 | unlock_new_inode(inode); |
| 356 | if (S_ISDIR(lower_inode->i_mode)) | 357 | d_add(ecryptfs_dentry, inode); |
| 357 | goto out; | ||
| 358 | if (S_ISLNK(lower_inode->i_mode)) | ||
| 359 | goto out; | ||
| 360 | if (special_file(lower_inode->i_mode)) | ||
| 361 | goto out; | 358 | goto out; |
| 359 | } | ||
| 362 | /* Released in this function */ | 360 | /* Released in this function */ |
| 363 | page_virt = kmem_cache_zalloc(ecryptfs_header_cache_2, GFP_USER); | 361 | page_virt = kmem_cache_zalloc(ecryptfs_header_cache_2, GFP_USER); |
| 364 | if (!page_virt) { | 362 | if (!page_virt) { |
| 365 | printk(KERN_ERR "%s: Cannot kmem_cache_zalloc() a page\n", | 363 | printk(KERN_ERR "%s: Cannot kmem_cache_zalloc() a page\n", |
| 366 | __func__); | 364 | __func__); |
| 367 | rc = -ENOMEM; | 365 | rc = -ENOMEM; |
| 366 | make_bad_inode(inode); | ||
| 368 | goto out; | 367 | goto out; |
| 369 | } | 368 | } |
| 370 | rc = ecryptfs_get_lower_file(ecryptfs_dentry); | 369 | rc = ecryptfs_get_lower_file(ecryptfs_dentry, inode); |
| 371 | if (rc) { | 370 | if (rc) { |
| 372 | printk(KERN_ERR "%s: Error attempting to initialize " | 371 | printk(KERN_ERR "%s: Error attempting to initialize " |
| 373 | "the lower file for the dentry with name " | 372 | "the lower file for the dentry with name " |
| 374 | "[%s]; rc = [%d]\n", __func__, | 373 | "[%s]; rc = [%d]\n", __func__, |
| 375 | ecryptfs_dentry->d_name.name, rc); | 374 | ecryptfs_dentry->d_name.name, rc); |
| 375 | make_bad_inode(inode); | ||
| 376 | goto out_free_kmem; | 376 | goto out_free_kmem; |
| 377 | } | 377 | } |
| 378 | put_lower = 1; | 378 | put_lower = 1; |
| 379 | crypt_stat = &ecryptfs_inode_to_private( | 379 | crypt_stat = &ecryptfs_inode_to_private(inode)->crypt_stat; |
| 380 | ecryptfs_dentry->d_inode)->crypt_stat; | ||
| 381 | /* TODO: lock for crypt_stat comparison */ | 380 | /* TODO: lock for crypt_stat comparison */ |
| 382 | if (!(crypt_stat->flags & ECRYPTFS_POLICY_APPLIED)) | 381 | if (!(crypt_stat->flags & ECRYPTFS_POLICY_APPLIED)) |
| 383 | ecryptfs_set_default_sizes(crypt_stat); | 382 | ecryptfs_set_default_sizes(crypt_stat); |
| 384 | rc = ecryptfs_read_and_validate_header_region(page_virt, | 383 | rc = ecryptfs_read_and_validate_header_region(page_virt, inode); |
| 385 | ecryptfs_dentry->d_inode); | ||
| 386 | if (rc) { | 384 | if (rc) { |
| 387 | memset(page_virt, 0, PAGE_CACHE_SIZE); | 385 | memset(page_virt, 0, PAGE_CACHE_SIZE); |
| 388 | rc = ecryptfs_read_and_validate_xattr_region(page_virt, | 386 | rc = ecryptfs_read_and_validate_xattr_region(page_virt, |
| 389 | ecryptfs_dentry); | 387 | inode); |
| 390 | if (rc) { | 388 | if (rc) { |
| 391 | rc = 0; | 389 | rc = 0; |
| 392 | goto out_free_kmem; | 390 | goto unlock_inode; |
| 393 | } | 391 | } |
| 394 | crypt_stat->flags |= ECRYPTFS_METADATA_IN_XATTR; | 392 | crypt_stat->flags |= ECRYPTFS_METADATA_IN_XATTR; |
| 395 | } | 393 | } |
| 396 | ecryptfs_i_size_init(page_virt, ecryptfs_dentry->d_inode); | 394 | ecryptfs_i_size_init(page_virt, inode); |
| 395 | unlock_inode: | ||
| 396 | if (inode->i_state & I_NEW) | ||
| 397 | unlock_new_inode(inode); | ||
| 398 | d_add(ecryptfs_dentry, inode); | ||
| 397 | out_free_kmem: | 399 | out_free_kmem: |
| 398 | kmem_cache_free(ecryptfs_header_cache_2, page_virt); | 400 | kmem_cache_free(ecryptfs_header_cache_2, page_virt); |
| 399 | goto out; | 401 | goto out; |
| @@ -403,7 +405,7 @@ out_put: | |||
| 403 | d_drop(ecryptfs_dentry); | 405 | d_drop(ecryptfs_dentry); |
| 404 | out: | 406 | out: |
| 405 | if (put_lower) | 407 | if (put_lower) |
| 406 | ecryptfs_put_lower_file(ecryptfs_dentry->d_inode); | 408 | ecryptfs_put_lower_file(inode); |
| 407 | return rc; | 409 | return rc; |
| 408 | } | 410 | } |
| 409 | 411 | ||
| @@ -843,7 +845,7 @@ static int truncate_upper(struct dentry *dentry, struct iattr *ia, | |||
| 843 | lower_ia->ia_valid &= ~ATTR_SIZE; | 845 | lower_ia->ia_valid &= ~ATTR_SIZE; |
| 844 | return 0; | 846 | return 0; |
| 845 | } | 847 | } |
| 846 | rc = ecryptfs_get_lower_file(dentry); | 848 | rc = ecryptfs_get_lower_file(dentry, inode); |
| 847 | if (rc) | 849 | if (rc) |
| 848 | return rc; | 850 | return rc; |
| 849 | crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat; | 851 | crypt_stat = &ecryptfs_inode_to_private(dentry->d_inode)->crypt_stat; |
| @@ -999,7 +1001,7 @@ static int ecryptfs_setattr(struct dentry *dentry, struct iattr *ia) | |||
| 999 | 1001 | ||
| 1000 | mount_crypt_stat = &ecryptfs_superblock_to_private( | 1002 | mount_crypt_stat = &ecryptfs_superblock_to_private( |
| 1001 | dentry->d_sb)->mount_crypt_stat; | 1003 | dentry->d_sb)->mount_crypt_stat; |
| 1002 | rc = ecryptfs_get_lower_file(dentry); | 1004 | rc = ecryptfs_get_lower_file(dentry, inode); |
| 1003 | if (rc) { | 1005 | if (rc) { |
| 1004 | mutex_unlock(&crypt_stat->cs_mutex); | 1006 | mutex_unlock(&crypt_stat->cs_mutex); |
| 1005 | goto out; | 1007 | goto out; |
diff --git a/fs/ecryptfs/main.c b/fs/ecryptfs/main.c index 7c697abab396..943a4f55ed6d 100644 --- a/fs/ecryptfs/main.c +++ b/fs/ecryptfs/main.c | |||
| @@ -135,12 +135,12 @@ static int ecryptfs_init_lower_file(struct dentry *dentry, | |||
| 135 | return rc; | 135 | return rc; |
| 136 | } | 136 | } |
| 137 | 137 | ||
| 138 | int ecryptfs_get_lower_file(struct dentry *dentry) | 138 | int ecryptfs_get_lower_file(struct dentry *dentry, struct inode *inode) |
| 139 | { | 139 | { |
| 140 | struct ecryptfs_inode_info *inode_info = | 140 | struct ecryptfs_inode_info *inode_info; |
| 141 | ecryptfs_inode_to_private(dentry->d_inode); | ||
| 142 | int count, rc = 0; | 141 | int count, rc = 0; |
| 143 | 142 | ||
| 143 | inode_info = ecryptfs_inode_to_private(inode); | ||
| 144 | mutex_lock(&inode_info->lower_file_mutex); | 144 | mutex_lock(&inode_info->lower_file_mutex); |
| 145 | count = atomic_inc_return(&inode_info->lower_file_count); | 145 | count = atomic_inc_return(&inode_info->lower_file_count); |
| 146 | if (WARN_ON_ONCE(count < 1)) | 146 | if (WARN_ON_ONCE(count < 1)) |
