Date:   Tue, 4 Jan 2022 21:40:20 +0000
From:   Al Viro <viro@zeniv.linux.org.uk>
To:     Qinghua Jin <qhjin.dev@gmail.com>
Cc:     Colin Ian King <colin.king@canonical.com>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] vfs: fix bug when opening a file with O_DIRECT on a
 file system that does not support it will leave an empty file
Message-ID: <YdS+xJMYbBMJBTcH@zeniv-ca.linux.org.uk>
References: <20220104094217.99187-1-qhjin.dev@gmail.com>
 <YdRdBcUAp7Mgp4pV@zeniv-ca.linux.org.uk>
 <CACL7WENRAkZKo5sx5HvRJ_e9KXmM-SyOutb8BAthJzfH1b2vDA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACL7WENRAkZKo5sx5HvRJ_e9KXmM-SyOutb8BAthJzfH1b2vDA@mail.gmail.com>
Sender: Al Viro <viro@ftp.linux.org.uk>
Precedence: bulk

On Wed, Jan 05, 2022 at 05:23:30AM +0800, Qinghua Jin wrote:
> OK, thanks.
> So, how to resolve this bug? I think we should check the param before
> creating the inode and dentry. Maybe the best place is in the lookup_open
> where the last component get created, and the parent inode and child inode
> is in the same filesystem, so I think we can use
> dir_inode->i_mapping->a_ops->direct_IO to check the O_DIRECT param. The
> code looks like this:
>   if((open_flag & O_CREAT) && (open_flag & O_DIRECT)) {
>     if (!dir_inode->i_mapping || !dir_inode->i_mapping->a_ops ||
>             !dir_inode->i_mapping->a_ops->direct_IO)
>                 return ERR_PTR(-EINVAL);
>     }
> should do the work.

Why would it?  Seriously, why would directories' use of page cache resemble that
by the regular files on the same fs?  Sure, for minixfs we have that - directory
contents there happens to be stored in exact same way as for regular files.
For many other filesystem types it's not true.

Incidentally, how would an inode possibly get NULL ->i_mapping?  Or NULL
->a_ops, for that matter (IOW, the existing check is also partially
cargo-culted)...