ZFS refuses to private read/write map of immutable files on Linux

[Low-power]

Motivation and Context

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1052472

Description

Private read/write mapping can't be used to modify the underlying files, such
mapping shouldn't be rejected because the files having the immutable flag.

How Has This Been Tested?

This change has been tested with Linux 6.5 (linux-image-6.5.0-1-powerpc64 from
Debian) in a root-on-ZFS configuration. Immtuable files are still appear being
immutable.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[behlendorf]

@amotin this appears to be a linux specific issue, but can you double check the FreeBSD side.

[amotin]

@behlendorf I am not familiar with the mmap() code, but I hope that in case of MAP_PRIVATE no write requests will ever reach ZFS to allow modification of immutable files.

[behlendorf]

Well you're right this isn't safe. I doubled checked and sure enough the generic filemap_page_mkwrite() we're registering doesn't check for this. I believe we'll need to register our own custom struct generic_file_vm_ops and callbacks to properly support this.

[Low-power]

Just found #2724.

[behlendorf]

Private read/write mapping can't be used to modify the mapped files, so they will remain be immutable.

Where does this get enforced if we remove this check?

[Low-power]

From my testings, ext4 doesn't check this on mmap(2) call at all.

If a process successfully R/W **open(2)**ed a file from ext4, and the file was later made immutable, the process can still mmap(2) the file with MAP_SHARED; but any attempt to write the mapped memory will fail and the process will receive a SIGBUS.

I'm not sure whether will this get checked in ZFS too.

[behlendorf]

I'm not sure whether will this get checked in ZFS too.

Based on my reading of the code it currently won't be, so for now we have to keep this check. To do this right we'll need to register our own custom struct generic_file_vm_ops and callbacks. This is all doable but a bit more work.

[Low-power]

I think returning an EPREM for MAP_SHARED R/W mmap(2) request is good enough if the file was later made immutable. The only issue with this check I can imagine is that will make appending an append-only file with mmap(2) impossible; but I can't come up a real world usecase for it, so let's keep this check just yet.

On other hand this bug of rejecting MAP_PRIVATE R/W request is quiet annoying, as it breaks my usecase where I had on other kernels/filesystems for years. I really hope this simple fix be applied, and be backported into a stable branch ASAP.

[Low-power]

Some more testings I have done with the fix applied

R/W and MAP_SHARED

Because for this type of mmap(2) to work, the file descriptor must be available for writing, the file must be made immutable between open(2) and mmap(2) for the corresponding ZFS code be reached.

root@debiansid-be:~/src# grep -F zfs /proc/mounts 
zr/ROOT/debiansid-be / zfs rw,relatime,xattr,noacl 0 0
zr/usr/home /usr/home zfs rw,noatime,xattr,noacl 0 0
zr /zr zfs rw,noatime,xattr,noacl 0 0
zr/usr/src/openzfs /usr/src/openzfs zfs rw,noatime,xattr,noacl 0 0
zr/ROOT/debianwheezy-i386 /i386 zfs rw,noatime,xattr,noacl 0 0
root@debiansid-be:~/src# cat mmap-rw-test.c 
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char **argv) {
	if(argc != 2) {
		return -1;
	}
	int fd = open(argv[1], O_RDWR);
	if(fd == -1) {
		perror(argv[1]);
		return 1;
	}
	sleep(10);
	void *map = mmap(NULL, 8192, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
	if(map == MAP_FAILED) {
		perror("mmap");
		return 1;
	}
	fprintf(stderr, "%p\n", map);
	strcpy(map, "Hello world\n");
	sleep(10);
	fputs("written\n", stderr);
	return 0;
}
root@debiansid-be:~/src# gcc -Wall -O1 mmap-rw-test.c -o mmap-rw-test
root@debiansid-be:~/src# echo "This file is immutable" > /test-file
root@debiansid-be:~/src# chattr +i /test-file 
root@debiansid-be:~/src# lsattr /test-file 
----i----------------- /test-file
root@debiansid-be:~/src# ./mmap-rw-test /test-file 
/test-file: Operation not permitted
root@debiansid-be:~/src# chattr -i /test-file 
root@debiansid-be:~/src# ./mmap-rw-test /test-file &
[1] 179397
root@debiansid-be:~/src# chattr +i /test-file 
root@debiansid-be:~/src# mmap: Operation not permitted

[1]+  Exit 1                  ./mmap-rw-test /test-file

This shows the mmap(2) failed with EPERM as expected.

R/W and MAP_PRIVATE

Private R/W mapping didn't require the file descriptor be available for writing, so this tested is being performed with both O_RDWR and O_RDONLY to open(2) call.

root@debiansid-be:~/src# cat mmap-rw-test.c 
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char **argv) {
	if(argc != 2) {
		return -1;
	}
	int fd = open(argv[1], O_RDWR);
	if(fd == -1) {
		perror(argv[1]);
		return 1;
	}
	sleep(10);
	void *map = mmap(NULL, 8192, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
	if(map == MAP_FAILED) {
		perror("mmap");
		return 1;
	}
	fprintf(stderr, "%p\n", map);
	strcpy(map, "Hello world\n");
	sleep(10);
	fputs("written\n", stderr);
	return 0;
}
root@debiansid-be:~/src# gcc -Wall -O1 mmap-rw-test.c -o mmap-rw-test
root@debiansid-be:~/src# ./mmap-rw-test /test-file 
/test-file: Operation not permitted
root@debiansid-be:~/src# chattr -i /test-file 
root@debiansid-be:~/src# ./mmap-rw-test /test-file &
[1] 3654
root@debiansid-be:~/src# chattr +i /test-file 
root@debiansid-be:~/src# 0x7fff86f20000
written

[1]+  Done                    ./mmap-rw-test /test-file
root@debiansid-be:~/src# cat /test-file 
This file is immutable
root@debiansid-be:~/src# sed -i s/O_RDWR/O_RDONLY/ mmap-rw-test.c
root@debiansid-be:~/src# gcc -Wall -O1 mmap-rw-test.c -o mmap-rw-test
root@debiansid-be:~/src# ./mmap-rw-test /test-file 
0x7fff978e0000
written
root@debiansid-be:~/src# cat /test-file 
This file is immutable

R/W and MAP_PRIVATE on buggy ZFS version

This test shows that in current unfixed versions of ZFS, even the file was **open(2)**ed with O_RDONLY (no reason to use O_RDWR with MAP_PRIVATE anyways), private R/W mapping is still unnecessarily rejected for no good.

root@debiansid-be:~/src# cat mmap-rw-test.c
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>
#include <stdio.h>
#include <string.h>

int main(int argc, char **argv) {
	if(argc != 2) {
		return -1;
	}
	int fd = open(argv[1], O_RDONLY);
	if(fd == -1) {
		perror(argv[1]);
		return 1;
	}
	sleep(10);
	void *map = mmap(NULL, 8192, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
	if(map == MAP_FAILED) {
		perror("mmap");
		return 1;
	}
	fprintf(stderr, "%p\n", map);
	strcpy(map, "Hello world\n");
	sleep(10);
	fputs("written\n", stderr);
	return 0;
}
root@debiansid-be:~/src# gcc -Wall -O1 mmap-rw-test.c -o mmap-rw-test
root@debiansid-be:~/src# lsattr /test-file 
----i----------------- /test-file
root@debiansid-be:~/src# ./mmap-rw-test /test-file 
mmap: Operation not permitted

[behlendorf]

@Low-power thanks for whipping up those test cases. I ran through it myself locally and was confirmed that in the VM_PRIVATE case nothing written to the mapping hits disk. That addresses my concern above so this LGTM. Thanks.