ext4: fix invalid free tracking in ext4_xattr_move_to_block()

In ext4_xattr_move_to_block(), the value of the extended attribute which we need to move to an external block may be allocated by kvmalloc() if the value is stored in an external inode. So at the end of the function the code tried to check if this was the case by testing entry->e_value_inum. However, at this point, the pointer to the xattr entry is no longer valid, because it was removed from the original location where it had been stored. So we could end up calling kvfree() on a pointer which was not allocated by kvmalloc(); or we could also potentially leak memory by not freeing the buffer when it should be freed. Fix this by storing whether it should be freed in a separate variable.

Theodore Ts'o committed 3 years ago

Commit b87c7cd1 parent 463808f1 file · +3 −2Browse files

Unified · Split

fs/ext4/xattr.c+3−2

@@ -2614,6 +2614,7 @@ static int ext4_xattr_move_to_block(handle_t handle, struct inode inode, · 2613 unmodified lines
2614	.in_inode = !!entry->e_value_inum,	2614	.in_inode = !!entry->e_value_inum,
2615	};	2615	};
2616	struct ext4_xattr_ibody_header *header = IHDR(inode, raw_inode);	2616	struct ext4_xattr_ibody_header *header = IHDR(inode, raw_inode);
		2617	int needs_kvfree = 0;
2617	int error;	2618	int error;
2618		2619
2619	is = kzalloc(sizeof(struct ext4_xattr_ibody_find), GFP_NOFS);	2620	is = kzalloc(sizeof(struct ext4_xattr_ibody_find), GFP_NOFS);
@@ -2636,7 +2637,7 @@ static int ext4_xattr_move_to_block(handle_t handle, struct inode inode, · 16 unmodified lines
2636	error = -ENOMEM;	2637	error = -ENOMEM;
2637	goto out;	2638	goto out;
2638	}	2639	}
2639		2640	needs_kvfree = 1;
2640	error = ext4_xattr_inode_get(inode, entry, buffer, value_size);	2641	error = ext4_xattr_inode_get(inode, entry, buffer, value_size);
2641	if (error)	2642	if (error)
2642	goto out;	2643	goto out;
@@ -2675,7 +2676,7 @@ static int ext4_xattr_move_to_block(handle_t handle, struct inode inode, · 32 unmodified lines
2675		2676
2676	out:	2677	out:
2677	kfree(b_entry_name);	2678	kfree(b_entry_name);
2678	if (entry->e_value_inum && buffer)	2679	if (needs_kvfree && buffer)
2679	kvfree(buffer);	2680	kvfree(buffer);
2680	if (is)	2681	if (is)
2681	brelse(is->iloc.bh);	2682	brelse(is->iloc.bh);