[Ocfs2-devel] [PATCH] Ocfs2: do not allow fallocate on dir file

[Mark Fasheh]

On Thu, Mar 10, 2011 at 02:00:33PM +0800, Jiaju Zhang wrote:
> On Thu, Mar 10, 2011 at 12:25 PM, Mark Fasheh <mfasheh at suse.com> wrote:
> > On Thu, Mar 10, 2011 at 10:46:33AM +0800, Tao Ma wrote:
> >> On 03/10/2011 10:27 AM, Sunil Mushran wrote:
> >> > On 03/09/2011 03:01 PM, Mark Fasheh wrote:
> >> >> On Fri, Mar 04, 2011 at 03:10:33PM +0800, Li Dongyang wrote:
> >> >>> @@ -1870,6 +1870,11 @@ static int __ocfs2_change_file_space(struct file *file, struct inode *inode,
> >> >>>                   goto out_inode_unlock;
> >> >>>           }
> >> >>>
> >> >>> + if (!S_ISREG(inode->i_mode)) {
> >> >>> +         ret = -EINVAL;
> >> >>> +         goto out_inode_unlock;
> >> >>> + }
> >> >> You might want to move the check into ocfs2_fallocate to mirror what
> >> >> ocfs2_change_file_space() does. Otherwise, looks like a good catch.
> >> >>
> >> >
> >> > Tristan pointed out that this check was already in ocfs2_change_file_space().
> >> > So this patch should not be required. Am I missing something?
> >> This patch is needed since the check is in ocfs2_change_file_space. But
> >> ocfs2_fallocate calls __ocfs2_change_file_space directly.
> >
> > Right.
> 
> It seems it is not right:-) If I read the code correctly, although
> ocfs2_fallocate calls __ocfs2_change_file_space directly, however,
> ocfs2_fallocate is a file operation but not an inode operation,
> ocfs2_fallocate can only be registered when that file is a regular
> file, so this patch is not needed.

Please review the code in fs/open.c:do_fallocate()

        /*
         * Let individual file system decide if it supports preallocation
         * for directories or not.
         */
        if (!S_ISREG(inode->i_mode) && !S_ISDIR(inode->i_mode))
                return -ENODEV;

So, sys_fallocate is leaving the decision up to the file system. Therefore I
think your patch is needed after all :)
	--Mark

--
Mark Fasheh