From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Chinner Subject: Re: [PATCH v3] Check for immutable/append flag in fallocate path Date: Wed, 9 Mar 2011 12:30:41 +1100 Message-ID: <20110309013041.GH1956@dastard> References: <4D6221B8.9040303@gmail.com> <4D6F5473.2070709@gmail.com> <4D720469.1010101@gmail.com> <20110308051121.GE1956@dastard> <92E5B019-756F-4CBE-829C-724564D4D650@dilger.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marco Stornelli , Linux Kernel , Linux FS Devel To: Andreas Dilger Return-path: Content-Disposition: inline In-Reply-To: <92E5B019-756F-4CBE-829C-724564D4D650@dilger.ca> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Mon, Mar 07, 2011 at 10:38:37PM -0700, Andreas Dilger wrote: > On 2011-03-07, at 10:11 PM, Dave Chinner wrote: > > On Sat, Mar 05, 2011 at 10:37:45AM +0100, Marco Stornelli wrote: > >> From: Marco Stornelli > >> > >> In the fallocate path the kernel doesn't check for the immutable/append > >> flag. It's possible to have a race condition in this scenario: an > >> application open a file in read/write and it does something, meanwhile > >> root set the immutable flag on the file, the application at that point > >> can call fallocate with success. In addition, we don't allow to do any > >> unreserve operation on an append only file but only the reserve one. > >> > >> Signed-off-by: Marco Stornelli > >> --- > >> Patch is against 2.6.38-rc7 > >> > >> ChangeLog: > >> v3: Modified do_fallocate instead of every single fs > >> v2: Added the check for append-only file for XFS > >> v1: First draft > >> > >> --- open.c.orig 2011-03-01 22:55:12.000000000 +0100 > >> +++ open.c 2011-03-04 15:28:43.000000000 +0100 > >> @@ -233,6 +233,14 @@ int do_fallocate(struct file *file, int > >> > >> if (!(file->f_mode & FMODE_WRITE)) > >> return -EBADF; > >> + > >> + /* It's not possible punch hole on append only file */ > >> + if (mode & FALLOC_FL_PUNCH_HOLE && IS_APPEND(inode)) > >> + return -EPERM; > > > > Seeing as I didn't get an answer in before you reposted, I still > > think punching an append-only file is a valid thing to want to do. > > > > I've seen this done in the past for application-level transaction > > journal files. The journal file is append only so new transactions > > can only be written at the end of the file i.e. you cannot overwrite > > (and therefore corrupt) existing transactions. However, once a > > transaction is complete and the changes flushed to disk, the > > transaction is punched out of the file to zero the range so it > > doesn't get replayed during recovery after a system crash. > > To my thinking "append only" means just that - only new data can > be written at the end of the file, and existing data cannot be > modified. Allowing hole punch on such a file (e.g. range 0 .. ~0) > would allow erasing all of the data, entirely bypassing the > append-only flag. Not worth arguing over. XFS_IOC_UNRESVSP won't get changed, so the applications already doing this can just keep using that interface... Cheers, Dave. -- Dave Chinner david@fromorbit.com