Bug 270749

Summary:

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

Product:

Base System

Reporter:

Jamie Landeg-Jones <jamie>

Component:

kern

Assignee:

Alan Somers <asomers>

Status:

Closed FIXED

Severity:

Affects Many People

CC:

asomers, grahamperrin, jamie

Priority:

---

Flags:

asomers: mfc-stable14+
asomers: mfc-stable13+
asomers: mfc-stable12-

Version:

14.0-CURRENT

Hardware:

Any

OS:

Any

URL:

https://reviews.freebsd.org/D41925

Attachments:

Description	Flags
patch to check whether we have permissions to update atime	none

Description Jamie Landeg-Jones 2023-04-10 23:30:08 UTC

Created attachment 241405 [details]
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

Comment 1 Alan Somers freebsd_committer

2023-09-20 21:46:59 UTC

Note that this bug affects 13.1-RELEASE, 13.2-RELEASE, and the upcoming 14.0 .

Comment 2 commit-hook freebsd_committer

2023-09-21 14:03:18 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fb619c94c679e939496fe0cf94b8d2cba95e6e63

commit fb619c94c679e939496fe0cf94b8d2cba95e6e63
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-09-21 14:02:31 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    MFC after:      1 week
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D41925

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 42 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 85 ++++++++++++++++++++++++++++++
 4 files changed, 150 insertions(+), 6 deletions(-)

Comment 3 commit-hook freebsd_committer

2023-09-21 15:40:58 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e5236d25f2c0709acf3547e6af45f4bb4eec4f02

commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-21 15:38:06 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-09-21 15:40:49 +0000

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    MFC after:      1 week
    MFC with:       fb619c94c679e939496fe0cf94b8d2cba95e6e63
    Sponsored by:   Axcient

 tests/sys/fs/fusefs/default_permissions.cc | 1 -
 tests/sys/fs/fusefs/read.cc                | 2 --
 2 files changed, 3 deletions(-)

Comment 4 Jamie Landeg-Jones 2023-09-21 21:30:17 UTC

(In reply to Alan Somers from comment #1)

Thanks Alan!

Comment 5 commit-hook freebsd_committer

2023-10-03 01:11:15 UTC

A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=13f188ce0b51908c292cdbb85ef26962528af87b

commit 13f188ce0b51908c292cdbb85ef26962528af87b
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-03 01:10:13 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D41925

    (cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    Sponsored by:   Axcient

    (cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 41 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 83 ++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 6 deletions(-)

Comment 6 commit-hook freebsd_committer

2023-10-03 01:11:17 UTC

A commit in branch stable/14 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=13f188ce0b51908c292cdbb85ef26962528af87b

commit 13f188ce0b51908c292cdbb85ef26962528af87b
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-03 01:10:13 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D41925

    (cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    Sponsored by:   Axcient

    (cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 41 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 83 ++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 6 deletions(-)

Comment 7 commit-hook freebsd_committer

2023-10-03 21:47:06 UTC

A commit in branch releng/14.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=63c0b60063c7ce1022b07879e63fcef26fc0259a

commit 63c0b60063c7ce1022b07879e63fcef26fc0259a
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-03 21:45:28 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Approved by:    re (gjb)
    Differential Revision: https://reviews.freebsd.org/D41925

    (cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    Sponsored by:   Axcient
    Approved by:    re (gjb)

    (cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)
    (cherry picked from commit 13f188ce0b51908c292cdbb85ef26962528af87b)

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 41 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 83 ++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 6 deletions(-)

Comment 8 commit-hook freebsd_committer

2023-10-03 21:47:07 UTC

A commit in branch releng/14.0 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=63c0b60063c7ce1022b07879e63fcef26fc0259a

commit 63c0b60063c7ce1022b07879e63fcef26fc0259a
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-03 21:45:28 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Approved by:    re (gjb)
    Differential Revision: https://reviews.freebsd.org/D41925

    (cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    Sponsored by:   Axcient
    Approved by:    re (gjb)

    (cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)
    (cherry picked from commit 13f188ce0b51908c292cdbb85ef26962528af87b)

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 41 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 83 ++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 6 deletions(-)

Comment 9 commit-hook freebsd_committer

2023-10-05 17:35:04 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2d05cbe002726a28e11060a601d1877784f6d587

commit 2d05cbe002726a28e11060a601d1877784f6d587
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-05 16:16:11 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D41925

    (cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    Sponsored by:   Axcient

    (cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 41 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 83 ++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 6 deletions(-)

Comment 10 commit-hook freebsd_committer

2023-10-05 17:35:05 UTC

A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2d05cbe002726a28e11060a601d1877784f6d587

commit 2d05cbe002726a28e11060a601d1877784f6d587
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2023-09-20 21:37:31 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2023-10-05 16:16:11 +0000

    fusefs: fix some bugs updating atime during close

    When using cached attributes, we must update a file's atime during
    close, if it has been read since the last attribute refresh.  But,

    * Don't update atime if we lack write permissions to the file or if the
      file system is readonly.
    * If the daemon fails our atime update request for any reason, don't
      report this as a failure for VOP_CLOSE.

    PR:             270749
    Reported by:    Jamie Landeg-Jones <jamie@catflap.org>
    Sponsored by:   Axcient
    Reviewed by:    pfg
    Differential Revision: https://reviews.freebsd.org/D41925

    (cherry picked from commit fb619c94c679e939496fe0cf94b8d2cba95e6e63)

    fusefs: fix unused variables from fb619c94c67

    PR:             270749
    Reported by:    cy
    Sponsored by:   Axcient

    (cherry picked from commit e5236d25f2c0709acf3547e6af45f4bb4eec4f02)

 sys/fs/fuse/fuse_vnops.c                   | 27 ++++++++--
 tests/sys/fs/fusefs/access.cc              |  2 +-
 tests/sys/fs/fusefs/default_permissions.cc | 41 ++++++++++++++-
 tests/sys/fs/fusefs/read.cc                | 83 ++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+), 6 deletions(-)