linux-kernel - Re: [PATCH v2] Check for immutable 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]

Message-ID: <4D709FFC.6000107@gmail.com>
Date:	Fri, 04 Mar 2011 09:17:00 +0100
From:	Marco Stornelli <marco.stornelli@...il.com>
To:	Dave Chinner <david@...morbit.com>
CC:	Linux Kernel <linux-kernel@...r.kernel.org>,
	linux-ext4@...r.kernel.org, linux-btrfs@...r.kernel.org,
	cluster-devel@...hat.com, xfs@....sgi.com,
	Linux FS Devel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v2] Check for immutable flag in fallocate path

Hi Dave,

Il 03/03/2011 22:39, Dave Chinner ha scritto:
> WTF?  Why does append mode have any effect on whether we can punch
> holes in a file or not? There's no justification for adding this in
> the commit message. Why is it even in a patch that is for checking
> immutable inodes? What is the point of adding it, when all that will
> happen is people will switch to XFS_IOC_UNRESVSP which has never had
> this limitation?

So according to you, it's legal to do an "unreserve" operation on an
append-only file. It's not the same for me, but if the community said
that this is the right behavior then ok.

> 
> And this asks bigger questions - why would you allow preallocate
> anywhere but at or beyond EOF on an append mode inode? You can only
> append to the file, so if you're going to add limitations based on
> the append flag, you need to think this through a bit more....
> 

I don't understand this point. The theory of operation was:

1) we don't allow any operation (reserve/unreserve) on a immutable file;
2) we don't allow *unreserve* operation on an append-only file (this
check makes sense only for fs that support the unreserve operation).

> 
> Also, like Christoph said, these checks belong in the generic code,
> not in every filesystem. The same checks have to be made for every
> filesystem, so they should be done before calling out the
> filesystems regardless of what functionality the filesystem actually
> supports.
> 

This was related to the first point, if we remove it then it's ok to
check in a common code. Even if I think we should do the check under the
inode lock to avoid race between fallocate and setattr, isn't it?

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/