On 2019/3/28 下午9:53, David Sterba wrote:
> On Wed, Mar 20, 2019 at 02:37:17PM +0800, Qu Wenruo wrote:
>> @@ -5726,18 +5734,29 @@ struct inode *btrfs_lookup_dentry(struct inode *dir, struct dentry *dentry)
>> struct btrfs_root *root = BTRFS_I(dir)->root;
>> struct btrfs_root *sub_root = root;
>> struct btrfs_key location;
>> + u8 di_type = 0;
>> int index;
>> int ret = 0;
>>
>> if (dentry->d_name.len > BTRFS_NAME_LEN)
>> return ERR_PTR(-ENAMETOOLONG);
>>
>> - ret = btrfs_inode_by_name(dir, dentry, &location);
>> + ret = btrfs_inode_by_name(dir, dentry, &location, &di_type);
>> if (ret < 0)
>> return ERR_PTR(ret);
>>
>> if (location.type == BTRFS_INODE_ITEM_KEY) {
>> inode = btrfs_iget(dir->i_sb, &location, root, NULL);
>> +
>> + /* Do extra check against inode mode with di_type */
>> + if (btrfs_inode_type(inode) != di_type) {
>> + btrfs_crit(fs_info,
>> +"inode mode mismatch with dir: inode mode=0%o btrfs type=%u dir type=%u",
>> + inode->i_mode, btrfs_inode_type(inode),
>> + di_type);
>> + iput(inode);
>
> The iput here seems suspicious.
How? this pairs to that inode = btrfs_iget() line.
And as long as we hit this branch, we will return either a valid inode
or an error pointer.
>
>> + return ERR_PTR(-EUCLEAN);
>> + }
>> return inode;
>> }
>
> 5792 index = srcu_read_lock(&fs_info->subvol_srcu);
> 5793 ret = fixup_tree_root_location(fs_info, dir, dentry,
> 5794 &location, &sub_root);
> 5795 if (ret < 0) {
> 5796 if (ret != -ENOENT)
> 5797 inode = ERR_PTR(ret);
> 5798 else
> 5799 inode = new_simple_dir(dir->i_sb, &location, sub_root);
> 5800 } else {
> 5801 inode = btrfs_iget(dir->i_sb, &location, sub_root, NULL);
> 5802 }
>
> The iget happens after this block so either there's another one in the caller
> to pair, or the above iput is wrong.
This block is unrelated to above (location.type == BTRFS_INODE_ITEM_KEY)
branch.
Or did I miss something?
Thanks,
Qu
>
