[2/3,v2] fs: Prevent doing FALLOC_FL_ZERO_RANGE on append only file

Submitted by Lukas Czerner on April 15, 2014, 4:41 p.m.

Details

Message ID 1397580076-19826-2-git-send-email-lczerner@redhat.com
State Accepted, archived
Headers show

Commit Message

Lukas Czerner April 15, 2014, 4:41 p.m.
Currently punch hole and collapse range fallocate operation are not
allowed on append only file. This should be case for zero range as well.
Fix it by allowing only pure fallocate (possibly with keep size set).

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
v2: Change the condition to be future proof as suggested by hch

 fs/open.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Dave Chinner April 15, 2014, 10:02 p.m.
On Tue, Apr 15, 2014 at 06:41:15PM +0200, Lukas Czerner wrote:
> Currently punch hole and collapse range fallocate operation are not
> allowed on append only file. This should be case for zero range as well.
> Fix it by allowing only pure fallocate (possibly with keep size set).
> 
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> ---
> v2: Change the condition to be future proof as suggested by hch
> 
>  fs/open.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/open.c b/fs/open.c
> index 631aea81..fe48b2f 100644
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -254,11 +254,9 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
>  		return -EBADF;
>  
>  	/*
> -	 * It's not possible to punch hole or perform collapse range
> -	 * on append only file
> +	 * We can only allow pure fallocate on append only files
>  	 */
> -	if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE)
> -	    && IS_APPEND(inode))
> +	if (mode & ~FALLOC_FL_KEEP_SIZE && IS_APPEND(inode))

	if ((mode & ~FALLOC_FL_KEEP_SIZE) && IS_APPEND(inode))

gcc normally complains when you mix & and && in the same logic
statement without () to separate the logic. I agree with gcc here,
because the () indicate the intent of the logic and make it easy to
determine that the & and && haven't been mixed up or fat-fingered...

Cheers,

Dave.
Theodore Ts'o April 16, 2014, 2:51 a.m.
On Wed, Apr 16, 2014 at 08:02:20AM +1000, Dave Chinner wrote:
> On Tue, Apr 15, 2014 at 06:41:15PM +0200, Lukas Czerner wrote:
> > Currently punch hole and collapse range fallocate operation are not
> > allowed on append only file. This should be case for zero range as well.
> > Fix it by allowing only pure fallocate (possibly with keep size set).
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Thanks, updated with Dave's suggested added parenthesis.

						- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Czerner April 16, 2014, 8:29 a.m.
On Wed, 16 Apr 2014, Dave Chinner wrote:

> Date: Wed, 16 Apr 2014 08:02:20 +1000
> From: Dave Chinner <david@fromorbit.com>
> To: Lukas Czerner <lczerner@redhat.com>
> Cc: linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, xfs@oss.sgi.com
> Subject: Re: [PATCH 2/3 v2] fs: Prevent doing FALLOC_FL_ZERO_RANGE on append
>     only file
> 
> On Tue, Apr 15, 2014 at 06:41:15PM +0200, Lukas Czerner wrote:
> > Currently punch hole and collapse range fallocate operation are not
> > allowed on append only file. This should be case for zero range as well.
> > Fix it by allowing only pure fallocate (possibly with keep size set).
> > 
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> > ---
> > v2: Change the condition to be future proof as suggested by hch
> > 
> >  fs/open.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/open.c b/fs/open.c
> > index 631aea81..fe48b2f 100644
> > --- a/fs/open.c
> > +++ b/fs/open.c
> > @@ -254,11 +254,9 @@ int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
> >  		return -EBADF;
> >  
> >  	/*
> > -	 * It's not possible to punch hole or perform collapse range
> > -	 * on append only file
> > +	 * We can only allow pure fallocate on append only files
> >  	 */
> > -	if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE)
> > -	    && IS_APPEND(inode))
> > +	if (mode & ~FALLOC_FL_KEEP_SIZE && IS_APPEND(inode))
> 
> 	if ((mode & ~FALLOC_FL_KEEP_SIZE) && IS_APPEND(inode))
> 
> gcc normally complains when you mix & and && in the same logic
> statement without () to separate the logic. I agree with gcc here,
> because the () indicate the intent of the logic and make it easy to
> determine that the & and && haven't been mixed up or fat-fingered...

Yeah, I was thinking about this and then left it to operator
precedence. But having () in there is fine as well.

-Lukas

> 
> Cheers,
> 
> Dave.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/fs/open.c b/fs/open.c
index 631aea81..fe48b2f 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -254,11 +254,9 @@  int do_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
 		return -EBADF;
 
 	/*
-	 * It's not possible to punch hole or perform collapse range
-	 * on append only file
+	 * We can only allow pure fallocate on append only files
 	 */
-	if (mode & (FALLOC_FL_PUNCH_HOLE | FALLOC_FL_COLLAPSE_RANGE)
-	    && IS_APPEND(inode))
+	if (mode & ~FALLOC_FL_KEEP_SIZE && IS_APPEND(inode))
 		return -EPERM;
 
 	if (IS_IMMUTABLE(inode))