mmap of immutable file behaves differently on zfs vs. ext4/btrfs

[jdsa]

I use an application called Geeqie to view photos.

Geeqie cannot open files from zfs (0.6.3) when those files are marked immutable. However, it can open them when marked immutable on ext4 or btrfs.

I'm not a programmer but I believe I traced it to this line in the Geeqie source:

il->mapped_file = mmap(0, il->bytes_total, PROT_READ|PROT_WRITE, MAP_PRIVATE, load_fd, 0);

This returns MAP_FAILED on zfs, but works on the other filesystems.

The offending part appears to be the PROT_WRITE flag.

Should zfs behave as the others do here?

[ryao]

@jdsa A private mapping should be writeable, but the code does not currently support this. I suspect that this is a bug that we inherited from Solaris/Illumos, but I need to confirm it.

[behlendorf]

@jdsa Thanks for the very clear bug report. You've put your finger exactly on the issue, in zfs_map() there is the following check which results in the MAP_FAILED error you're seeing. You should see the same behavior for read-only and append-only files.

        if ((vm_flags & VM_WRITE) && (zp->z_pflags &
            (ZFS_IMMUTABLE | ZFS_READONLY | ZFS_APPENDONLY))) {
                ZFS_EXIT(zsb);
                return (SET_ERROR(EPERM));
        }

Should zfs behave as the others do here?

Maybe. Removing this check would be simple enough but we'd need to carefully look at the Linux VFS code to see if it will correctly prevent pages from being written too. If it doesn't then you could end up modifying a file even though it was marked immutable. There's also the issue that this is code that is common to ZFS on FreeBSD and Illumos. Changing the behavior here to be consistent with Linux would make the behavior different on those other platforms.

Does your application require the file to be opened with PROT_WRITE. If not, then the cleanest more portable fix would be for them to stop passing that flag.

[behlendorf]

From the mmap(2) man page, one should expect EACCES for append-only files. It doesn't discuss how file attributed should influence the behavior.

       EACCES A file descriptor refers to a non-regular file.  Or  MAP_PRIVATE  was  requested,
              but  fd  is  not open for reading.  Or MAP_SHARED was requested and PROT_WRITE is
              set, but fd is not open in read/write (O_RDWR) mode.  Or PROT_WRITE is  set,  but
              the file is append-only.

[ryao]

@behlendorf Here, we have mmap() called with PROT_WRITE with MAP_PRIVATE. The description of MAP_PRIVATE in the Single UNIX Specification suggests that this should work, as it explicits states that we do not modify the underlying file:

If an implementation cannot support the combination of access types specified by prot, the call to mmap() fails. An implementation may permit accesses other than those specified by prot; however, the implementation will not permit a write to succeed where PROT_WRITE has not been set or permit any access where PROT_NONE alone has been set. The implementation will support at least the following values of prot: PROT_NONE, PROT_READ, PROT_WRITE, and the inclusive OR of PROT_READ and PROT_WRITE. The file descriptor fildes will have been opened with read permission, regardless of the protection options specified. If PROT_WRITE is specified, the application must have opened the file descriptor fildes with write permission unless MAP_PRIVATE is specified in the flags parameter as described below.

http://pubs.opengroup.org/onlinepubs/7908799/xsh/mmap.html

That said, it would be interesting to find out how FreeBSD and Illumos handle this. I suspect that they do not permit this, but it is possible their VFS does some magic that eliminates the need for this codepath to be invoked.

[behlendorf]

FreeBSD and Illumos handle this

They handle this by returning an error from zfs_map(). That code is common between all the implementations.

If we remove this check there isn't anything in the ZFS filesystem driver which would prevent the file from being updated. We would need to add an aops callback to do something like prevent the page from being dirtied so it's never written. And it might be a little tricky because we'd need to tied that behavior to a specific file handle. Once a dbuf has been dirtied there's no existing infrastructure to determine how/why it was dirtied.

[jdsa]

Does your application require the file to be opened with PROT_WRITE. If not, then the cleanest more portable fix would be for them to stop passing that flag.

Seems to work fine without PROT_WRITE. Every other application I've tried can open immutable files normally, so what Geeqie is doing certainly isn't common. I'll put in a request with that project.

If it isn't a correctness issue with ZFS then I'm not bothered. It seems like an obscure thing to run into. Geeqie itself is old and infrequently maintained.

Thanks both of you for taking a look at this.

[ryao]

@behlendorf @jdsa I had a chat with Garret D'Amore, who has been working on POSIX compliance for the Illumos project. He and I are both of the opinion that this is a conformance issue. Fixing this particular conformance issue is a low priority compared to other work being done for 0.6.4, but I imagine that either @behlendorf or myself will try to tackle it for 0.6.4.

[stale]

This issue has been automatically marked as "stale" because it has not had any activity for a while. It will be closed in 90 days if no further activity occurs. Thank you for your contributions.

[scineram]

Is this issue now Linux specific? Because now zfs_map() is not used on FreeBSD AFAIK.

[behlendorf]

@scineram that's probably a fair way to characterize it. Let me tag this as a Linux specific issue for now and reopen it so we can continue to track it. I don't think anything has changed in this regard.

[Low-power]

It isn't just a conformance issue; it creates real problem:

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