All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Luís Henriques via Ocfs2-devel" <ocfs2-devel@oss.oracle.com>
To: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: linux-kernel@vger.kernel.org, ocfs2-devel@oss.oracle.com
Subject: Re: [Ocfs2-devel] [PATCH] ocfs2: check new file size on fallocate	call
Date: Wed, 31 May 2023 09:29:18 +0100	[thread overview]
Message-ID: <87fs7c3nvl.fsf@suse.de> (raw)
In-Reply-To: <810630b9-2021-01b3-1473-aa759174205e@linux.alibaba.com> (Joseph Qi's message of "Wed, 31 May 2023 14:00:12 +0800")

Joseph Qi <joseph.qi@linux.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@suse.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.

>>  	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,
-- 
Luís

>
> Thanks,
> Joseph
>
>> +		ret = inode_newsize_ok(inode, offset + len);
>> +		if (ret)
>> +			return ret;
>> +	}
>>  
>>  	if (mode & FALLOC_FL_PUNCH_HOLE)
>>  		cmd = OCFS2_IOC_UNRESVSP64;


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
https://oss.oracle.com/mailman/listinfo/ocfs2-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Luís Henriques" <lhenriques@suse.de>
To: Joseph Qi <joseph.qi@linux.alibaba.com>
Cc: Mark Fasheh <mark@fasheh.com>, Joel Becker <jlbec@evilplan.org>,
	Heming Zhao <heming.zhao@suse.com>,
	ocfs2-devel@oss.oracle.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] ocfs2: check new file size on fallocate call
Date: Wed, 31 May 2023 09:29:18 +0100	[thread overview]
Message-ID: <87fs7c3nvl.fsf@suse.de> (raw)
In-Reply-To: <810630b9-2021-01b3-1473-aa759174205e@linux.alibaba.com> (Joseph Qi's message of "Wed, 31 May 2023 14:00:12 +0800")

Joseph Qi <joseph.qi@linux.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@suse.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.

>>  	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,
-- 
Luís

>
> Thanks,
> Joseph
>
>> +		ret = inode_newsize_ok(inode, offset + len);
>> +		if (ret)
>> +			return ret;
>> +	}
>>  
>>  	if (mode & FALLOC_FL_PUNCH_HOLE)
>>  		cmd = OCFS2_IOC_UNRESVSP64;


  reply	other threads:[~2023-05-31 15:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-29 15:26 [Ocfs2-devel] [PATCH] ocfs2: check new file size on fallocate call Luís Henriques via Ocfs2-devel
2023-05-29 15:26 ` Luís Henriques
2023-05-31  3:32 ` [Ocfs2-devel] " Mark Fasheh via Ocfs2-devel
2023-05-31  3:32   ` Mark Fasheh
2023-05-31  6:00 ` [Ocfs2-devel] " Joseph Qi via Ocfs2-devel
2023-05-31  6:00   ` Joseph Qi
2023-05-31  8:29   ` Luís Henriques via Ocfs2-devel [this message]
2023-05-31  8:29     ` Luís Henriques
2023-05-31  8:31     ` [Ocfs2-devel] " Joseph Qi via Ocfs2-devel
2023-05-31  8:31       ` Joseph Qi
2023-05-31  8:32 ` [Ocfs2-devel] " Joseph Qi via Ocfs2-devel
2023-05-31  8:32   ` Joseph Qi
2023-05-31 22:11 ` [Ocfs2-devel] " Andrew Morton via Ocfs2-devel
2023-05-31 22:11   ` Andrew Morton
2023-06-01  1:23   ` Joseph Qi via Ocfs2-devel
2023-06-01  1:23     ` Joseph Qi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87fs7c3nvl.fsf@suse.de \
    --to=ocfs2-devel@oss.oracle.com \
    --cc=joseph.qi@linux.alibaba.com \
    --cc=lhenriques@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.