[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <301cdaee-0b73-67b5-75d8-0cbad93b9736@linux.alibaba.com>
Date: Wed, 31 May 2023 16:31:19 +0800
From: Joseph Qi <joseph.qi@...ux.alibaba.com>
To: Luís Henriques <lhenriques@...e.de>
Cc: Mark Fasheh <mark@...heh.com>, Joel Becker <jlbec@...lplan.org>,
Heming Zhao <heming.zhao@...e.com>, ocfs2-devel@....oracle.com,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ocfs2: check new file size on fallocate call
On 5/31/23 4:29 PM, Luís Henriques wrote:
> Joseph Qi <joseph.qi@...ux.alibaba.com> writes:
>
>> On 5/29/23 11:26 PM, Luís Henriques wrote:
>>> When changing a file size with fallocate() the new size isn't being
>>> checked. In particular, the FSIZE ulimit isn't being checked, which makes
>>> fstest generic/228 fail. Simply adding a call to inode_newsize_ok() fixes
>>> this issue.
>>>
>>> Signed-off-by: Luís Henriques <lhenriques@...e.de>
>>> ---
>>> fs/ocfs2/file.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
>>> index efb09de4343d..b173c36bcab3 100644
>>> --- a/fs/ocfs2/file.c
>>> +++ b/fs/ocfs2/file.c
>>> @@ -2100,14 +2100,20 @@ static long ocfs2_fallocate(struct file *file, int mode, loff_t offset,
>>> struct ocfs2_space_resv sr;
>>> int change_size = 1;
>>> int cmd = OCFS2_IOC_RESVSP64;
>>> + int ret = 0;
>>>
>>> if (mode & ~(FALLOC_FL_KEEP_SIZE | FALLOC_FL_PUNCH_HOLE))
>>> return -EOPNOTSUPP;
>>
>> This means we only support keep-size and pouch_hole.
>> And it seems pouch_hole will also imply keep-size.
>
> I think you're forgetting about mode = 0, which is also valid. And the
> default '0' will allow size to be changed.
>
Oops... You are right.
>>> if (!ocfs2_writes_unwritten_extents(osb))
>>> return -EOPNOTSUPP;
>>>
>>> - if (mode & FALLOC_FL_KEEP_SIZE)
>>> + if (mode & FALLOC_FL_KEEP_SIZE) {
>>> change_size = 0;
>>> + } else {
>>
>> Seems this will be a dead branch?
>
> Again, you need to consider '0' as a valid mode value. If you run
> generic/228 without this patch you'll see that test failing because it
> *does* hit this branch.
>
> Cheers,
Powered by blists - more mailing lists