linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Sterba <dsterba@suse.cz>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	linux-ext4@vger.kernel.org, linux-btrfs@vger.kernel.org,
	linux-mm@kvack.org
Subject: Re: [PATCH v2 0/8] vfs: make immutable files actually immutable
Date: Tue, 30 Apr 2019 17:46:23 +0200	[thread overview]
Message-ID: <20190430154622.GA20156@twin.jikos.cz> (raw)
In-Reply-To: <155552786671.20411.6442426840435740050.stgit@magnolia>

On Wed, Apr 17, 2019 at 12:04:26PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> The chattr(1) manpage has this to say about the immutable bit that
> system administrators can set on files:
> 
> "A file with the 'i' attribute cannot be modified: it cannot be deleted
> or renamed, no link can be created to this file, most of the file's
> metadata can not be modified, and the file can not be opened in write
> mode."
> 
> Given the clause about how the file 'cannot be modified', it is
> surprising that programs holding writable file descriptors can continue
> to write to and truncate files after the immutable flag has been set,
> but they cannot call other things such as utimes, fallocate, unlink,
> link, setxattr, or reflink.
> 
> Since the immutable flag is only settable by administrators, resolve
> this inconsistent behavior in favor of the documented behavior -- once
> the flag is set, the file cannot be modified, period.

The manual page leaves the case undefined, though the word 'modified'
can be interpreted in the same sense as 'mtime' ie. modifying the file
data. The enumerated file operations that don't work on an immutable
file suggest that it's more like the 'ctime',  ie. (state) changes are
forbidden.

Tthe patchset makes some sense, but it changes the semantics a bit. From
'not changed but still modified' to 'neither changed nor modified'. It
starts to sound like a word game, but I think both are often used
interchangeably in the language. See the changelog of 1/8 where you used
them in the other meaning regarding ctime and mtime.

I personally doubt there's a real use of the undefined case, though
something artificial like 'a process opens a fd, sets up file in a very
specific way, sets immutable and hands the fd to an unprivileged
process' can be made up. The overhead of the new checks seems to be
small so performance is not the concern here.

Overall, I don't see a strong reason for either semantics. As long as
it's documented possibly with some of the corner cases described in more
detail, fine.

      parent reply	other threads:[~2019-04-30 15:45 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-17 19:04 [PATCH v2 0/8] vfs: make immutable files actually immutable Darrick J. Wong
2019-04-17 19:04 ` [PATCH 1/8] mm/fs: don't allow writes to immutable files Darrick J. Wong
2019-04-26 18:17   ` Brian Foster
2019-06-10  1:43   ` Theodore Ts'o
2019-06-10  1:51   ` Theodore Ts'o
2019-06-10  4:41     ` Darrick J. Wong
2019-06-10 13:14       ` Theodore Ts'o
2019-06-10 16:09         ` Darrick J. Wong
2019-06-10 20:41           ` Theodore Ts'o
2019-06-11  3:26             ` Darrick J. Wong
2019-06-11  4:01             ` Darrick J. Wong
2019-04-17 19:04 ` [PATCH 2/8] xfs: unlock inode when xfs_ioctl_setattr_get_trans can't get transaction Darrick J. Wong
2019-04-26 18:17   ` Brian Foster
2019-04-17 19:04 ` [PATCH 3/8] xfs: flush page mappings as part of setting immutable Darrick J. Wong
2019-04-26 18:18   ` Brian Foster
2019-04-17 19:04 ` [PATCH 4/8] xfs: refactor setflags to use setattr code directly Darrick J. Wong
2019-04-17 19:05 ` [PATCH 5/8] xfs: clean up xfs_merge_ioc_xflags Darrick J. Wong
2019-04-17 19:05 ` [PATCH 6/8] xfs: don't allow most setxattr to immutable files Darrick J. Wong
2019-04-17 19:05 ` [PATCH 7/8] btrfs: don't allow any modifications to an immutable file Darrick J. Wong
2019-04-17 19:05 ` [PATCH 8/8] ext4: " Darrick J. Wong
2019-04-30 15:46 ` David Sterba [this message]

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=20190430154622.GA20156@twin.jikos.cz \
    --to=dsterba@suse.cz \
    --cc=darrick.wong@oracle.com \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-xfs@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).