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)) |