[Bug 270749] [FUSEFS] File close() failures relating to attempted atime update

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=270749

            Bug ID: 270749
           Summary: [FUSEFS] File close() failures relating to attempted
                    atime update
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Only Me
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: jamie@catflap.org
 Attachment #241405 text/plain
         mime type:

Created attachment 241405
  --> https://bugs.freebsd.org/bugzilla/attachment.cgi?id=241405&action=edit
patch to check whether we have permissions to update atime

There is a bug in fusefs, discovered by ThomasWaldmann@github relating to
closing files on fusefs filesystems.

See: https://github.com/borgbackup/borg/issues/6871

I tracked it down to:

title:     fusefs: update atime on reads when using cached attributes
commit:    0bade34633f997c22f5e4e0931df0d534f560a38
author     Alan Somers <asomers@FreeBSD.org> 2021-11-29 01:53:31 +0000
committer  Alan Somers <asomers@FreeBSD.org> 2021-12-14 22:15:53 +0000
link:     
https://cgit.freebsd.org/src/commit/sys/fs/fuse?h=stable/13&id=0bade34633f997c22f5e4e0931df0d534f560a38

The problem only started after that commit - I've tested current (as of today)
and the issue still exists there.

This is my take of what is going on:

Basically, it appears that on a fusefs mounted filesystem, a file close fails
when fuse tries to update the "atime" of a file that it doesn't have
write-access to.

It doesn't matter if the filesystem is mounted read-only - the effect is the
same.

For example, if a file is opened and read, and then closed, all subsequent
closes to that file fail until the filesystem is remounted. - Even closes for
opens that don't themselves update atime fail - presumably because fuse still
has this metadata pending.

The following test demonstrates this. "file-close-check" is a simple c program
that does an fopen/fclose then another fopen/fclose, then fopen/fread 100
bytes/fclose, followed by a final fopen/fclose

These are the results. On the remote system, "/tmp/test[12]" are just small
text files, one owned by me, one not:

 4 -rw-r--r--   1 jamie            wheel  -     1,626 10 Apr 19:58 test1
 4 -rw-r--r--   1 root             wheel  -     1,626 10 Apr 18:56 test2

root@catwalk:/tmp # mkdir xx

root@catwalk:/tmp # sshfs jamie@catflap.org:/tmp xx

root@catwalk:/tmp # file-close-check xx/test?
xx/test1   | open: ok, close: ok | open: ok, close: ok | open: ok, read: 100
bytes, close: ok | open: ok, close: ok
xx/test2   | open: ok, close: ok | open: ok, close: ok | open: ok, read: 100
bytes, close: Permission denied | open: ok, close: Permission denied

Then run the same command again:

root@catwalk:/tmp # file-close-check xx/test?
xx/test1   | open: ok, close: ok | open: ok, close: ok | open: ok, read: 100
bytes, close: ok | open: ok, close: ok
xx/test2   | open: ok, close: Permission denied | open: ok, close: Permission
denied | open: ok, read: 100 bytes, close: Permission denied | open: ok, close:
Permission denied

From what I can see, in file "sys/fs/fuse/fuse_vnops.c", it does indeed check
if the data flush succeeded, and if so, and there has been an atime update, it
then tries to set that:

        err = fuse_flush(vp, cred, pid, fflag);
        if (err == 0 && (fvdat->flag & FN_ATIMECHANGE)) {
                struct vattr vap;

                VATTR_NULL(&vap);
                vap.va_atime = fvdat->cached_attrs.va_atime;
                err = fuse_internal_setattr(vp, &vap, td, NULL);
        }

However, if there is no data to write, the flush succeeds even if there is no
write access to the file, causing the fuse_internal_setattr to then fail.

The attached patch "fixes" the problem, by adding the "checkparam" code to this
section too, but I don't know if it's the right fix, or if it breaks what the
commit does, so someone with more fusefs experience needs to check it.

Cheers, Jamie

-- 
You are receiving this mail because:
You are the assignee for the bug.