Re: [PATCh v2 9/9] btrfs: inode: Verify inode mode to avoid NULL pointer dereference

[David Sterba <dsterba@suse.cz>]

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.

> +			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.