mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	ext4: fix races in ext4_sync_parent()
Fix problems if fsync() races against a rename of a parent directory
as pointed out by Al Viro in his own inimitable way:
>While we are at it, could somebody please explain what the hell is ext4
>doing in
>static int ext4_sync_parent(struct inode *inode)
>{
>        struct writeback_control wbc;
>        struct dentry *dentry = NULL;
>        int ret = 0;
>
>        while (inode && ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY)) {
>                ext4_clear_inode_state(inode, EXT4_STATE_NEWENTRY);
>                dentry = list_entry(inode->i_dentry.next,
>                                    struct dentry, d_alias);
>                if (!dentry || !dentry->d_parent || !dentry->d_parent->d_inode)
>                        break;
>                inode = dentry->d_parent->d_inode;
>                ret = sync_mapping_buffers(inode->i_mapping);
>                ...
>Note that dentry obviously can't be NULL there.  dentry->d_parent is never
>NULL.  And dentry->d_parent would better not be negative, for crying out
>loud!  What's worse, there's no guarantees that dentry->d_parent will
>remain our parent over that sync_mapping_buffers() *and* that inode won't
>just be freed under us (after rename() and memory pressure leading to
>eviction of what used to be our dentry->d_parent)......
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>
			
			
This commit is contained in:
		
							parent
							
								
									29ae07b702
								
							
						
					
					
						commit
						d59729f4e7
					
				
					 1 changed files with 21 additions and 5 deletions
				
			
		| 
						 | 
				
			
			@ -129,15 +129,30 @@ static int ext4_sync_parent(struct inode *inode)
 | 
			
		|||
{
 | 
			
		||||
	struct writeback_control wbc;
 | 
			
		||||
	struct dentry *dentry = NULL;
 | 
			
		||||
	struct inode *next;
 | 
			
		||||
	int ret = 0;
 | 
			
		||||
 | 
			
		||||
	while (inode && ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY)) {
 | 
			
		||||
	if (!ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY))
 | 
			
		||||
		return 0;
 | 
			
		||||
	inode = igrab(inode);
 | 
			
		||||
	while (ext4_test_inode_state(inode, EXT4_STATE_NEWENTRY)) {
 | 
			
		||||
		ext4_clear_inode_state(inode, EXT4_STATE_NEWENTRY);
 | 
			
		||||
		dentry = list_entry(inode->i_dentry.next,
 | 
			
		||||
				    struct dentry, d_alias);
 | 
			
		||||
		if (!dentry || !dentry->d_parent || !dentry->d_parent->d_inode)
 | 
			
		||||
		dentry = NULL;
 | 
			
		||||
		spin_lock(&inode->i_lock);
 | 
			
		||||
		if (!list_empty(&inode->i_dentry)) {
 | 
			
		||||
			dentry = list_first_entry(&inode->i_dentry,
 | 
			
		||||
						  struct dentry, d_alias);
 | 
			
		||||
			dget(dentry);
 | 
			
		||||
		}
 | 
			
		||||
		spin_unlock(&inode->i_lock);
 | 
			
		||||
		if (!dentry)
 | 
			
		||||
			break;
 | 
			
		||||
		inode = dentry->d_parent->d_inode;
 | 
			
		||||
		next = igrab(dentry->d_parent->d_inode);
 | 
			
		||||
		dput(dentry);
 | 
			
		||||
		if (!next)
 | 
			
		||||
			break;
 | 
			
		||||
		iput(inode);
 | 
			
		||||
		inode = next;
 | 
			
		||||
		ret = sync_mapping_buffers(inode->i_mapping);
 | 
			
		||||
		if (ret)
 | 
			
		||||
			break;
 | 
			
		||||
| 
						 | 
				
			
			@ -148,6 +163,7 @@ static int ext4_sync_parent(struct inode *inode)
 | 
			
		|||
		if (ret)
 | 
			
		||||
			break;
 | 
			
		||||
	}
 | 
			
		||||
	iput(inode);
 | 
			
		||||
	return ret;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue