linux-kernel - Re: [PATCH v3] Check for immutable/append flag in fallocate path

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 8 Mar 2011 08:35:56 +0100
From:	Marco Stornelli <marco.stornelli@...il.com>
To:	Andreas Dilger <adilger.kernel@...ger.ca>
Cc:	Dave Chinner <david@...morbit.com>,
	Linux Kernel <linux-kernel@...r.kernel.org>,
	Linux FS Devel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v3] Check for immutable/append flag in fallocate path

2011/3/8 Andreas Dilger <adilger.kernel@...ger.ca>:
> 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 <marco.stornelli@...il.com>
>>>
>>> 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 <marco.stornelli@...il.com>
>>> ---
>>> 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.
>
> Cheers, Andreas
>

I quite agree with Andreas.

Marco
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/