[PATCH v2] tmpfs: enforce the immutable flag on open files

[PATCH v2] tmpfs: enforce the immutable flag on open files

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 4803 bytes --]

This useful behaviour is implemented for most filesystems,
and wants to be implemented for every filesystem, quoth ref:
  There is general agreement that we should standardize all file systems
  to prevent modifications even for files that were opened at the time
  the immutable flag is set.  Eventually, a change to enforce this at
  the VFS layer should be landing in mainline.

References: commit 02b016ca7f99 ("ext4: enforce the immutable flag on
 open files")
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
v1: https://lore.kernel.org/linux-fsdevel/znhu3eyffewvvhleewehuvod2wrf4tz6vxrouoakiarjtxt5uy@tarta.nabijaczleweli.xyz/t/#u

shmem_page_mkwrite()'s return 0; falls straight into do_page_mkwrite()'s
	if (unlikely(!(ret & VM_FAULT_LOCKED))) {
		folio_lock(folio);
Given the unlikely, is it better to folio_lock(folio); return VM_FAULT_LOCKED; instead?

/ext4# uname -a
Linux tarta 6.18.0-10912-g416f99c3b16f-dirty #1 SMP PREEMPT_DYNAMIC Sat Dec  6 12:14:41 CET 2025 x86_64 GNU/Linux
/ext4# while sleep 1; do echo $$; done > file &
[1] 262
/ext4# chattr +i file
/ext4# sh: line 25: echo: write error: Operation not permitted
sh: line 25: echo: write error: Operation not permitted
sh: line 25: echo: write error: Operation not permitted
sh: line 25: echo: write error: Operation not permitted
fg
while sleep 1; do
    echo $$;
done > file
^C
/ext4# mount -t tmpfs tmpfs /tmp
/ext4# cd /tmp
/tmp# while sleep 1; do echo $$; done > file &
[1] 284
/tmp# chattr +i file
/tmp# sh: line 35: echo: write error: Operation not permitted
sh: line 35: echo: write error: Operation not permitted
sh: line 35: echo: write error: Operation not permitted

$ cat test.c
#include <unistd.h>
#include <fcntl.h>
#include <sys/ioctl.h>
#include <linux/fs.h>
#include <sys/mman.h>
int main(int, char **argv) {
	int fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0666);
	ftruncate(fd, 1024 * 1024);
	char *addr = mmap(NULL, 1024 * 1024, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
	addr[0] = 0x69;
	int attrs = FS_IMMUTABLE_FL;
	ioctl(3, FS_IOC_SETFLAGS, &attrs);
	addr[1024 * 1024 - 1] = 0x69;
}

# strace ./test /tmp/file
execve("./test", ["./test", "/tmp/file"], 0x7ffc720bead8 /* 22 vars */) = 0
...
openat(AT_FDCWD, "/tmp/file", O_RDWR|O_CREAT|O_TRUNC, 0666) = 3
ftruncate(3, 1048576)                   = 0
mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0x7f09bbf2a000
ioctl(3, FS_IOC_SETFLAGS, [FS_IMMUTABLE_FL]) = 0
--- SIGBUS {si_signo=SIGBUS, si_code=BUS_ADRERR, si_addr=0x7f09bc029fff} ---
+++ killed by SIGBUS +++
Bus error
# tr -d \\0 < /tmp/file; echo
i

 mm/shmem.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/mm/shmem.c b/mm/shmem.c
index d578d8e765d7..432935f79f35 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1294,6 +1294,14 @@ static int shmem_setattr(struct mnt_idmap *idmap,
 	bool update_mtime = false;
 	bool update_ctime = true;
 
+	if (unlikely(IS_IMMUTABLE(inode)))
+		return -EPERM;
+
+	if (unlikely(IS_APPEND(inode) &&
+		     (attr->ia_valid & (ATTR_MODE | ATTR_UID |
+					ATTR_GID | ATTR_TIMES_SET))))
+		return -EPERM;
+
 	error = setattr_prepare(idmap, dentry, attr);
 	if (error)
 		return error;
@@ -2763,6 +2771,17 @@ static vm_fault_t shmem_fault(struct vm_fault *vmf)
 	return ret;
 }
 
+static vm_fault_t shmem_page_mkwrite(struct vm_fault *vmf)
+{
+	struct file *file = vmf->vma->vm_file;
+
+	if (unlikely(IS_IMMUTABLE(file_inode(file))))
+		return VM_FAULT_SIGBUS;
+
+	file_update_time(file);
+	return 0;
+}
+
 unsigned long shmem_get_unmapped_area(struct file *file,
 				      unsigned long uaddr, unsigned long len,
 				      unsigned long pgoff, unsigned long flags)
@@ -3475,6 +3494,10 @@ static ssize_t shmem_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
 	ret = generic_write_checks(iocb, from);
 	if (ret <= 0)
 		goto unlock;
+	if (unlikely(IS_IMMUTABLE(inode))) {
+		ret = -EPERM;
+		goto unlock;
+	}
 	ret = file_remove_privs(file);
 	if (ret)
 		goto unlock;
@@ -5286,6 +5309,7 @@ static const struct super_operations shmem_ops = {
 static const struct vm_operations_struct shmem_vm_ops = {
 	.fault		= shmem_fault,
 	.map_pages	= filemap_map_pages,
+	.page_mkwrite	= shmem_page_mkwrite,
 #ifdef CONFIG_NUMA
 	.set_policy     = shmem_set_policy,
 	.get_policy     = shmem_get_policy,
@@ -5295,6 +5319,7 @@ static const struct vm_operations_struct shmem_vm_ops = {
 static const struct vm_operations_struct shmem_anon_vm_ops = {
 	.fault		= shmem_fault,
 	.map_pages	= filemap_map_pages,
+	.page_mkwrite	= shmem_page_mkwrite,
 #ifdef CONFIG_NUMA
 	.set_policy     = shmem_set_policy,
 	.get_policy     = shmem_get_policy,
-- 
2.39.5

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] tmpfs: enforce the immutable flag on open files

[Andrew Morton]

On Mon, 8 Dec 2025 22:50:15 +0100 Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz> wrote:

> This useful behaviour is implemented for most filesystems,
> and wants to be implemented for every filesystem, quoth ref:
>   There is general agreement that we should standardize all file systems
>   to prevent modifications even for files that were opened at the time
>   the immutable flag is set.  Eventually, a change to enforce this at
>   the VFS layer should be landing in mainline.

This changes the VFS parts of tmpfs, not the MM parts.  So please
resend and also cc

linux-fsdevel@vger.kernel.org
Christian Brauner <brauner@kernel.org>

Re: [PATCH v2] tmpfs: enforce the immutable flag on open files

[Hugh Dickins]

[-- Attachment #1: Type: text/plain, Size: 5946 bytes --]

On Mon, 8 Dec 2025, Ahelenia Ziemiańska wrote:

> This useful behaviour is implemented for most filesystems,
> and wants to be implemented for every filesystem, quoth ref:
>   There is general agreement that we should standardize all file systems
>   to prevent modifications even for files that were opened at the time
>   the immutable flag is set.  Eventually, a change to enforce this at
>   the VFS layer should be landing in mainline.
> 
> References: commit 02b016ca7f99 ("ext4: enforce the immutable flag on
>  open files")
> Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>

Sorry: thanks, but no thanks.

Supporting page_mkwrite() comes at a cost (an additional fault on first
write to a folio in a shared mmap).  It's important for space allocation
(and more) in the case of persistent writeback filesystems, but unwelcome
overhead in the case of tmpfs (and ramfs and hugetlbfs - others?).

tmpfs has always preferred not to support page_mkwrite(), and just fail
fstests generic/080: we shall not slow down to change that, without a
much stronger justification than "useful behaviour" which we've got
along well enough without.

But it is interesting that tmpfs supports IMMUTABLE, and passes all
the chattr fstests, without this patch.  Perhaps you should be adding
a new fstest, for tmpfs to fail: I won't thank you for that, but it
would be a fair response!

Hugh

> ---
> v1: https://lore.kernel.org/linux-fsdevel/znhu3eyffewvvhleewehuvod2wrf4tz6vxrouoakiarjtxt5uy@tarta.nabijaczleweli.xyz/t/#u
> 
> shmem_page_mkwrite()'s return 0; falls straight into do_page_mkwrite()'s
> 	if (unlikely(!(ret & VM_FAULT_LOCKED))) {
> 		folio_lock(folio);
> Given the unlikely, is it better to folio_lock(folio); return VM_FAULT_LOCKED; instead?
> 
> /ext4# uname -a
> Linux tarta 6.18.0-10912-g416f99c3b16f-dirty #1 SMP PREEMPT_DYNAMIC Sat Dec  6 12:14:41 CET 2025 x86_64 GNU/Linux
> /ext4# while sleep 1; do echo $$; done > file &
> [1] 262
> /ext4# chattr +i file
> /ext4# sh: line 25: echo: write error: Operation not permitted
> sh: line 25: echo: write error: Operation not permitted
> sh: line 25: echo: write error: Operation not permitted
> sh: line 25: echo: write error: Operation not permitted
> fg
> while sleep 1; do
>     echo $$;
> done > file
> ^C
> /ext4# mount -t tmpfs tmpfs /tmp
> /ext4# cd /tmp
> /tmp# while sleep 1; do echo $$; done > file &
> [1] 284
> /tmp# chattr +i file
> /tmp# sh: line 35: echo: write error: Operation not permitted
> sh: line 35: echo: write error: Operation not permitted
> sh: line 35: echo: write error: Operation not permitted
> 
> $ cat test.c
> #include <unistd.h>
> #include <fcntl.h>
> #include <sys/ioctl.h>
> #include <linux/fs.h>
> #include <sys/mman.h>
> int main(int, char **argv) {
> 	int fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0666);
> 	ftruncate(fd, 1024 * 1024);
> 	char *addr = mmap(NULL, 1024 * 1024, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> 	addr[0] = 0x69;
> 	int attrs = FS_IMMUTABLE_FL;
> 	ioctl(3, FS_IOC_SETFLAGS, &attrs);
> 	addr[1024 * 1024 - 1] = 0x69;
> }
> 
> # strace ./test /tmp/file
> execve("./test", ["./test", "/tmp/file"], 0x7ffc720bead8 /* 22 vars */) = 0
> ...
> openat(AT_FDCWD, "/tmp/file", O_RDWR|O_CREAT|O_TRUNC, 0666) = 3
> ftruncate(3, 1048576)                   = 0
> mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0x7f09bbf2a000
> ioctl(3, FS_IOC_SETFLAGS, [FS_IMMUTABLE_FL]) = 0
> --- SIGBUS {si_signo=SIGBUS, si_code=BUS_ADRERR, si_addr=0x7f09bc029fff} ---
> +++ killed by SIGBUS +++
> Bus error
> # tr -d \\0 < /tmp/file; echo
> i
> 
>  mm/shmem.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/mm/shmem.c b/mm/shmem.c
> index d578d8e765d7..432935f79f35 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1294,6 +1294,14 @@ static int shmem_setattr(struct mnt_idmap *idmap,
>  	bool update_mtime = false;
>  	bool update_ctime = true;
>  
> +	if (unlikely(IS_IMMUTABLE(inode)))
> +		return -EPERM;
> +
> +	if (unlikely(IS_APPEND(inode) &&
> +		     (attr->ia_valid & (ATTR_MODE | ATTR_UID |
> +					ATTR_GID | ATTR_TIMES_SET))))
> +		return -EPERM;
> +
>  	error = setattr_prepare(idmap, dentry, attr);
>  	if (error)
>  		return error;
> @@ -2763,6 +2771,17 @@ static vm_fault_t shmem_fault(struct vm_fault *vmf)
>  	return ret;
>  }
>  
> +static vm_fault_t shmem_page_mkwrite(struct vm_fault *vmf)
> +{
> +	struct file *file = vmf->vma->vm_file;
> +
> +	if (unlikely(IS_IMMUTABLE(file_inode(file))))
> +		return VM_FAULT_SIGBUS;
> +
> +	file_update_time(file);
> +	return 0;
> +}
> +
>  unsigned long shmem_get_unmapped_area(struct file *file,
>  				      unsigned long uaddr, unsigned long len,
>  				      unsigned long pgoff, unsigned long flags)
> @@ -3475,6 +3494,10 @@ static ssize_t shmem_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
>  	ret = generic_write_checks(iocb, from);
>  	if (ret <= 0)
>  		goto unlock;
> +	if (unlikely(IS_IMMUTABLE(inode))) {
> +		ret = -EPERM;
> +		goto unlock;
> +	}
>  	ret = file_remove_privs(file);
>  	if (ret)
>  		goto unlock;
> @@ -5286,6 +5309,7 @@ static const struct super_operations shmem_ops = {
>  static const struct vm_operations_struct shmem_vm_ops = {
>  	.fault		= shmem_fault,
>  	.map_pages	= filemap_map_pages,
> +	.page_mkwrite	= shmem_page_mkwrite,
>  #ifdef CONFIG_NUMA
>  	.set_policy     = shmem_set_policy,
>  	.get_policy     = shmem_get_policy,
> @@ -5295,6 +5319,7 @@ static const struct vm_operations_struct shmem_vm_ops = {
>  static const struct vm_operations_struct shmem_anon_vm_ops = {
>  	.fault		= shmem_fault,
>  	.map_pages	= filemap_map_pages,
> +	.page_mkwrite	= shmem_page_mkwrite,
>  #ifdef CONFIG_NUMA
>  	.set_policy     = shmem_set_policy,
>  	.get_policy     = shmem_get_policy,
> -- 
> 2.39.5

Re: [PATCH v2] tmpfs: enforce the immutable flag on open files

[Ahelenia Ziemiańska]

[-- Attachment #1: Type: text/plain, Size: 7111 bytes --]

On Mon, Dec 08, 2025 at 08:14:44PM -0800, Hugh Dickins wrote:
> On Mon, 8 Dec 2025, Ahelenia Ziemiańska wrote:
> > This useful behaviour is implemented for most filesystems,
> > and wants to be implemented for every filesystem, quoth ref:
> >   There is general agreement that we should standardize all file systems
> >   to prevent modifications even for files that were opened at the time
> >   the immutable flag is set.  Eventually, a change to enforce this at
> >   the VFS layer should be landing in mainline.
> > 
> > References: commit 02b016ca7f99 ("ext4: enforce the immutable flag on
> >  open files")
> > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> Sorry: thanks, but no thanks.
> 
> Supporting page_mkwrite() comes at a cost (an additional fault on first
> write to a folio in a shared mmap).  It's important for space allocation
> (and more) in the case of persistent writeback filesystems, but unwelcome
> overhead in the case of tmpfs (and ramfs and hugetlbfs - others?).

Yeah, from the way page_mkwrite() was implemented it looked like
enough of a pessimisation to be significant, and with how common
an operation this is, I kinda expected this result.

(I was also gonna post the same for ramfs,
 but it doesn't support FS_IOC_SETFLAGS attributes at all.)

> tmpfs has always preferred not to support page_mkwrite(), and just fail
> fstests generic/080: we shall not slow down to change that, without a
> much stronger justification than "useful behaviour" which we've got
> along well enough without.

How do we feel about just the VFS half of this,
i.e. open(WR)/chattr +i/write() = -EPERM?
That shouldn't have a performance impact.

(I'll admit that this is the behaviour I find to be useful,
 and I was surprised that the ext4 implementation also made mappings
 SIGBUS, but I implemented both out of an undue sense of completionism.)

> But it is interesting that tmpfs supports IMMUTABLE, and passes all
> the chattr fstests, without this patch.  Perhaps you should be adding
> a new fstest, for tmpfs to fail: I won't thank you for that, but it
> would be a fair response!

I rather think having IMMUTABLE but not atomically perfusing it
to file descriptions is worthy of a test failure.
The mmap behaviour, not so much.

> Hugh
> 
> > ---
> > v1: https://lore.kernel.org/linux-fsdevel/znhu3eyffewvvhleewehuvod2wrf4tz6vxrouoakiarjtxt5uy@tarta.nabijaczleweli.xyz/t/#u
> > 
> > shmem_page_mkwrite()'s return 0; falls straight into do_page_mkwrite()'s
> > 	if (unlikely(!(ret & VM_FAULT_LOCKED))) {
> > 		folio_lock(folio);
> > Given the unlikely, is it better to folio_lock(folio); return VM_FAULT_LOCKED; instead?
> > 
> > /ext4# uname -a
> > Linux tarta 6.18.0-10912-g416f99c3b16f-dirty #1 SMP PREEMPT_DYNAMIC Sat Dec  6 12:14:41 CET 2025 x86_64 GNU/Linux
> > /ext4# while sleep 1; do echo $$; done > file &
> > [1] 262
> > /ext4# chattr +i file
> > /ext4# sh: line 25: echo: write error: Operation not permitted
> > sh: line 25: echo: write error: Operation not permitted
> > sh: line 25: echo: write error: Operation not permitted
> > sh: line 25: echo: write error: Operation not permitted
> > fg
> > while sleep 1; do
> >     echo $$;
> > done > file
> > ^C
> > /ext4# mount -t tmpfs tmpfs /tmp
> > /ext4# cd /tmp
> > /tmp# while sleep 1; do echo $$; done > file &
> > [1] 284
> > /tmp# chattr +i file
> > /tmp# sh: line 35: echo: write error: Operation not permitted
> > sh: line 35: echo: write error: Operation not permitted
> > sh: line 35: echo: write error: Operation not permitted
> > 
> > $ cat test.c
> > #include <unistd.h>
> > #include <fcntl.h>
> > #include <sys/ioctl.h>
> > #include <linux/fs.h>
> > #include <sys/mman.h>
> > int main(int, char **argv) {
> > 	int fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0666);
> > 	ftruncate(fd, 1024 * 1024);
> > 	char *addr = mmap(NULL, 1024 * 1024, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> > 	addr[0] = 0x69;
> > 	int attrs = FS_IMMUTABLE_FL;
> > 	ioctl(3, FS_IOC_SETFLAGS, &attrs);
> > 	addr[1024 * 1024 - 1] = 0x69;
> > }
> > 
> > # strace ./test /tmp/file
> > execve("./test", ["./test", "/tmp/file"], 0x7ffc720bead8 /* 22 vars */) = 0
> > ...
> > openat(AT_FDCWD, "/tmp/file", O_RDWR|O_CREAT|O_TRUNC, 0666) = 3
> > ftruncate(3, 1048576)                   = 0
> > mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0x7f09bbf2a000
> > ioctl(3, FS_IOC_SETFLAGS, [FS_IMMUTABLE_FL]) = 0
> > --- SIGBUS {si_signo=SIGBUS, si_code=BUS_ADRERR, si_addr=0x7f09bc029fff} ---
> > +++ killed by SIGBUS +++
> > Bus error
> > # tr -d \\0 < /tmp/file; echo
> > i
> > 
> >  mm/shmem.c | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> > 
> > diff --git a/mm/shmem.c b/mm/shmem.c
> > index d578d8e765d7..432935f79f35 100644
> > --- a/mm/shmem.c
> > +++ b/mm/shmem.c
> > @@ -1294,6 +1294,14 @@ static int shmem_setattr(struct mnt_idmap *idmap,
> >  	bool update_mtime = false;
> >  	bool update_ctime = true;
> >  
> > +	if (unlikely(IS_IMMUTABLE(inode)))
> > +		return -EPERM;
> > +
> > +	if (unlikely(IS_APPEND(inode) &&
> > +		     (attr->ia_valid & (ATTR_MODE | ATTR_UID |
> > +					ATTR_GID | ATTR_TIMES_SET))))
> > +		return -EPERM;
> > +
> >  	error = setattr_prepare(idmap, dentry, attr);
> >  	if (error)
> >  		return error;
> > @@ -2763,6 +2771,17 @@ static vm_fault_t shmem_fault(struct vm_fault *vmf)
> >  	return ret;
> >  }
> >  
> > +static vm_fault_t shmem_page_mkwrite(struct vm_fault *vmf)
> > +{
> > +	struct file *file = vmf->vma->vm_file;
> > +
> > +	if (unlikely(IS_IMMUTABLE(file_inode(file))))
> > +		return VM_FAULT_SIGBUS;
> > +
> > +	file_update_time(file);
> > +	return 0;
> > +}
> > +
> >  unsigned long shmem_get_unmapped_area(struct file *file,
> >  				      unsigned long uaddr, unsigned long len,
> >  				      unsigned long pgoff, unsigned long flags)
> > @@ -3475,6 +3494,10 @@ static ssize_t shmem_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> >  	ret = generic_write_checks(iocb, from);
> >  	if (ret <= 0)
> >  		goto unlock;
> > +	if (unlikely(IS_IMMUTABLE(inode))) {
> > +		ret = -EPERM;
> > +		goto unlock;
> > +	}
> >  	ret = file_remove_privs(file);
> >  	if (ret)
> >  		goto unlock;
> > @@ -5286,6 +5309,7 @@ static const struct super_operations shmem_ops = {
> >  static const struct vm_operations_struct shmem_vm_ops = {
> >  	.fault		= shmem_fault,
> >  	.map_pages	= filemap_map_pages,
> > +	.page_mkwrite	= shmem_page_mkwrite,
> >  #ifdef CONFIG_NUMA
> >  	.set_policy     = shmem_set_policy,
> >  	.get_policy     = shmem_get_policy,
> > @@ -5295,6 +5319,7 @@ static const struct vm_operations_struct shmem_vm_ops = {
> >  static const struct vm_operations_struct shmem_anon_vm_ops = {
> >  	.fault		= shmem_fault,
> >  	.map_pages	= filemap_map_pages,
> > +	.page_mkwrite	= shmem_page_mkwrite,
> >  #ifdef CONFIG_NUMA
> >  	.set_policy     = shmem_set_policy,
> >  	.get_policy     = shmem_get_policy,
> > -- 
> > 2.39.5


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2] tmpfs: enforce the immutable flag on open files

[Darrick J. Wong]

On Tue, Dec 09, 2025 at 03:00:35PM +0100, Ahelenia Ziemiańska wrote:
> On Mon, Dec 08, 2025 at 08:14:44PM -0800, Hugh Dickins wrote:
> > On Mon, 8 Dec 2025, Ahelenia Ziemiańska wrote:
> > > This useful behaviour is implemented for most filesystems,
> > > and wants to be implemented for every filesystem, quoth ref:
> > >   There is general agreement that we should standardize all file systems
> > >   to prevent modifications even for files that were opened at the time
> > >   the immutable flag is set.  Eventually, a change to enforce this at
> > >   the VFS layer should be landing in mainline.
> > > 
> > > References: commit 02b016ca7f99 ("ext4: enforce the immutable flag on
> > >  open files")
> > > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> > Sorry: thanks, but no thanks.
> > 
> > Supporting page_mkwrite() comes at a cost (an additional fault on first
> > write to a folio in a shared mmap).  It's important for space allocation
> > (and more) in the case of persistent writeback filesystems, but unwelcome
> > overhead in the case of tmpfs (and ramfs and hugetlbfs - others?).
> 
> Yeah, from the way page_mkwrite() was implemented it looked like
> enough of a pessimisation to be significant, and with how common
> an operation this is, I kinda expected this result.
> 
> (I was also gonna post the same for ramfs,
>  but it doesn't support FS_IOC_SETFLAGS attributes at all.)
> 
> > tmpfs has always preferred not to support page_mkwrite(), and just fail
> > fstests generic/080: we shall not slow down to change that, without a
> > much stronger justification than "useful behaviour" which we've got
> > along well enough without.
> 
> How do we feel about just the VFS half of this,
> i.e. open(WR)/chattr +i/write() = -EPERM?
> That shouldn't have a performance impact.
> 
> (I'll admit that this is the behaviour I find to be useful,
>  and I was surprised that the ext4 implementation also made mappings
>  SIGBUS, but I implemented both out of an undue sense of completionism.)
> 
> > But it is interesting that tmpfs supports IMMUTABLE, and passes all
> > the chattr fstests, without this patch.  Perhaps you should be adding
> > a new fstest, for tmpfs to fail: I won't thank you for that, but it
> > would be a fair response!
> 
> I rather think having IMMUTABLE but not atomically perfusing it
> to file descriptions is worthy of a test failure.
> The mmap behaviour, not so much.

If tmpfs isn't going to implement the same FS_FLAG_IMMUTABLE behaviors
as all the other filesystems then you should remove support for setting
the flag at all and leave a comment that minimizing fault latencies was
more important to the maintainer, so that nobody gets any smart ideas
about adding it back in.  Better not to advertise support than to have
quirky sh*t all over the kernel.

--D

> > Hugh
> > 
> > > ---
> > > v1: https://lore.kernel.org/linux-fsdevel/znhu3eyffewvvhleewehuvod2wrf4tz6vxrouoakiarjtxt5uy@tarta.nabijaczleweli.xyz/t/#u
> > > 
> > > shmem_page_mkwrite()'s return 0; falls straight into do_page_mkwrite()'s
> > > 	if (unlikely(!(ret & VM_FAULT_LOCKED))) {
> > > 		folio_lock(folio);
> > > Given the unlikely, is it better to folio_lock(folio); return VM_FAULT_LOCKED; instead?
> > > 
> > > /ext4# uname -a
> > > Linux tarta 6.18.0-10912-g416f99c3b16f-dirty #1 SMP PREEMPT_DYNAMIC Sat Dec  6 12:14:41 CET 2025 x86_64 GNU/Linux
> > > /ext4# while sleep 1; do echo $$; done > file &
> > > [1] 262
> > > /ext4# chattr +i file
> > > /ext4# sh: line 25: echo: write error: Operation not permitted
> > > sh: line 25: echo: write error: Operation not permitted
> > > sh: line 25: echo: write error: Operation not permitted
> > > sh: line 25: echo: write error: Operation not permitted
> > > fg
> > > while sleep 1; do
> > >     echo $$;
> > > done > file
> > > ^C
> > > /ext4# mount -t tmpfs tmpfs /tmp
> > > /ext4# cd /tmp
> > > /tmp# while sleep 1; do echo $$; done > file &
> > > [1] 284
> > > /tmp# chattr +i file
> > > /tmp# sh: line 35: echo: write error: Operation not permitted
> > > sh: line 35: echo: write error: Operation not permitted
> > > sh: line 35: echo: write error: Operation not permitted
> > > 
> > > $ cat test.c
> > > #include <unistd.h>
> > > #include <fcntl.h>
> > > #include <sys/ioctl.h>
> > > #include <linux/fs.h>
> > > #include <sys/mman.h>
> > > int main(int, char **argv) {
> > > 	int fd = open(argv[1], O_RDWR | O_CREAT | O_TRUNC, 0666);
> > > 	ftruncate(fd, 1024 * 1024);
> > > 	char *addr = mmap(NULL, 1024 * 1024, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
> > > 	addr[0] = 0x69;
> > > 	int attrs = FS_IMMUTABLE_FL;
> > > 	ioctl(3, FS_IOC_SETFLAGS, &attrs);
> > > 	addr[1024 * 1024 - 1] = 0x69;
> > > }
> > > 
> > > # strace ./test /tmp/file
> > > execve("./test", ["./test", "/tmp/file"], 0x7ffc720bead8 /* 22 vars */) = 0
> > > ...
> > > openat(AT_FDCWD, "/tmp/file", O_RDWR|O_CREAT|O_TRUNC, 0666) = 3
> > > ftruncate(3, 1048576)                   = 0
> > > mmap(NULL, 1048576, PROT_READ|PROT_WRITE, MAP_SHARED, 3, 0) = 0x7f09bbf2a000
> > > ioctl(3, FS_IOC_SETFLAGS, [FS_IMMUTABLE_FL]) = 0
> > > --- SIGBUS {si_signo=SIGBUS, si_code=BUS_ADRERR, si_addr=0x7f09bc029fff} ---
> > > +++ killed by SIGBUS +++
> > > Bus error
> > > # tr -d \\0 < /tmp/file; echo
> > > i
> > > 
> > >  mm/shmem.c | 25 +++++++++++++++++++++++++
> > >  1 file changed, 25 insertions(+)
> > > 
> > > diff --git a/mm/shmem.c b/mm/shmem.c
> > > index d578d8e765d7..432935f79f35 100644
> > > --- a/mm/shmem.c
> > > +++ b/mm/shmem.c
> > > @@ -1294,6 +1294,14 @@ static int shmem_setattr(struct mnt_idmap *idmap,
> > >  	bool update_mtime = false;
> > >  	bool update_ctime = true;
> > >  
> > > +	if (unlikely(IS_IMMUTABLE(inode)))
> > > +		return -EPERM;
> > > +
> > > +	if (unlikely(IS_APPEND(inode) &&
> > > +		     (attr->ia_valid & (ATTR_MODE | ATTR_UID |
> > > +					ATTR_GID | ATTR_TIMES_SET))))
> > > +		return -EPERM;
> > > +
> > >  	error = setattr_prepare(idmap, dentry, attr);
> > >  	if (error)
> > >  		return error;
> > > @@ -2763,6 +2771,17 @@ static vm_fault_t shmem_fault(struct vm_fault *vmf)
> > >  	return ret;
> > >  }
> > >  
> > > +static vm_fault_t shmem_page_mkwrite(struct vm_fault *vmf)
> > > +{
> > > +	struct file *file = vmf->vma->vm_file;
> > > +
> > > +	if (unlikely(IS_IMMUTABLE(file_inode(file))))
> > > +		return VM_FAULT_SIGBUS;
> > > +
> > > +	file_update_time(file);
> > > +	return 0;
> > > +}
> > > +
> > >  unsigned long shmem_get_unmapped_area(struct file *file,
> > >  				      unsigned long uaddr, unsigned long len,
> > >  				      unsigned long pgoff, unsigned long flags)
> > > @@ -3475,6 +3494,10 @@ static ssize_t shmem_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> > >  	ret = generic_write_checks(iocb, from);
> > >  	if (ret <= 0)
> > >  		goto unlock;
> > > +	if (unlikely(IS_IMMUTABLE(inode))) {
> > > +		ret = -EPERM;
> > > +		goto unlock;
> > > +	}
> > >  	ret = file_remove_privs(file);
> > >  	if (ret)
> > >  		goto unlock;
> > > @@ -5286,6 +5309,7 @@ static const struct super_operations shmem_ops = {
> > >  static const struct vm_operations_struct shmem_vm_ops = {
> > >  	.fault		= shmem_fault,
> > >  	.map_pages	= filemap_map_pages,
> > > +	.page_mkwrite	= shmem_page_mkwrite,
> > >  #ifdef CONFIG_NUMA
> > >  	.set_policy     = shmem_set_policy,
> > >  	.get_policy     = shmem_get_policy,
> > > @@ -5295,6 +5319,7 @@ static const struct vm_operations_struct shmem_vm_ops = {
> > >  static const struct vm_operations_struct shmem_anon_vm_ops = {
> > >  	.fault		= shmem_fault,
> > >  	.map_pages	= filemap_map_pages,
> > > +	.page_mkwrite	= shmem_page_mkwrite,
> > >  #ifdef CONFIG_NUMA
> > >  	.set_policy     = shmem_set_policy,
> > >  	.get_policy     = shmem_get_policy,
> > > -- 
> > > 2.39.5
> 

Re: [PATCH v2] tmpfs: enforce the immutable flag on open files

[Hugh Dickins]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

On Tue, 9 Dec 2025, Ahelenia Ziemiańska wrote:
> On Mon, Dec 08, 2025 at 08:14:44PM -0800, Hugh Dickins wrote:
> > On Mon, 8 Dec 2025, Ahelenia Ziemiańska wrote:
> > > This useful behaviour is implemented for most filesystems,
> > > and wants to be implemented for every filesystem, quoth ref:
> > >   There is general agreement that we should standardize all file systems
> > >   to prevent modifications even for files that were opened at the time
> > >   the immutable flag is set.  Eventually, a change to enforce this at
> > >   the VFS layer should be landing in mainline.
> > > 
> > > References: commit 02b016ca7f99 ("ext4: enforce the immutable flag on
> > >  open files")
> > > Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
> > Sorry: thanks, but no thanks.
> > 
> > Supporting page_mkwrite() comes at a cost (an additional fault on first
> > write to a folio in a shared mmap).  It's important for space allocation
> > (and more) in the case of persistent writeback filesystems, but unwelcome
> > overhead in the case of tmpfs (and ramfs and hugetlbfs - others?).
> 
> Yeah, from the way page_mkwrite() was implemented it looked like
> enough of a pessimisation to be significant, and with how common
> an operation this is, I kinda expected this result.
> 
> (I was also gonna post the same for ramfs,
>  but it doesn't support FS_IOC_SETFLAGS attributes at all.)
> 
> > tmpfs has always preferred not to support page_mkwrite(), and just fail
> > fstests generic/080: we shall not slow down to change that, without a
> > much stronger justification than "useful behaviour" which we've got
> > along well enough without.
> 
> How do we feel about just the VFS half of this,
> i.e. open(WR)/chattr +i/write() = -EPERM?
> That shouldn't have a performance impact.

I don't think tmpfs should implement half of the ext4 behaviour (at
write time) and not the other half (at page_mkwrite time): it would
leave IMMUTABLE on tmpfs as neither guaranteeing immutabiity, nor
behaving in the way IMMUTABLE was traditionally supported.

I do think it's surprisingly asymmetic, that IMMUTABLE should forbid
opening for write, but holding open for write should not forbid
setting IMMUTABLE.  But that's the traditional behaviour, which
surprised you, and which those ext4 mods improve upon for ext4.

But I couldn't find any filesystem other than ext4 and f2fs
implementing it that way.  If the VFS and other filesystems agreed
to implement the stricter IMMUTABLE (I imagine relying on i_writecount),
then tmpfs would probably be glad to participate; but not go its own way.

Hugh