Bug 253500
| Summary: | [fusefs]: F_SETLKW is treated as F_SETLK and F_UNLCK is ignored | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | John Millikin <jmillikin> | ||||||
| Component: | kern | Assignee: | Alan Somers <asomers> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Only Me | CC: | asomers, jSML4ThWwBID69YC, kib, markj | ||||||
| Priority: | --- | Flags: | asomers:
mfc-stable13+
asomers: mfc-stable12+ asomers: mfc-stable11- |
||||||
| Version: | 12.2-RELEASE | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
The patch is wrong, of course. It breaks calling conventions for lf_advlockasync() which is the implementation for all advlock vops (except fuse and partially nfs). You need to adopt fuse to the interface provided by VOP. If there is missed information at the VOP call time, describe what sort of info is lost. Kib's right. The VOP_ADVLOCK interface isn't exactly the same as fcntl. It's not legal for op to be F_SETLKW. Instead, it should be F_SETLK and the flags field should contain F_WAIT. You can fix this in fuse_vnop_advlock . The second problem is also likely caused by fuse_vnop_advlock, but I don't have time to debug it right now. If you're planning to submit a fix, could you please also fix and/or extend the tests in tests/sys/fs/fusefs/locks.cc ? Created attachment 222867 [details]
Fixes two bugs in fuse_vnop_advlock
John, can you please test the attached patch?
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=929acdb19acb67cc0e6ee5439df98e28a84d4772 commit 929acdb19acb67cc0e6ee5439df98e28a84d4772 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2021-03-18 20:27:27 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2021-03-18 23:09:10 +0000 fusefs: fix two bugs regarding fcntl file locks 1) F_SETLKW (blocking) operations would be sent to the FUSE server as F_SETLK (non-blocking). 2) Release operations, F_SETLK with lk_type = F_UNLCK, would simply return EINVAL. PR: 253500 Reported by: John Millikin <jmillikin@gmail.com> MFC after: 2 weeks sys/fs/fuse/fuse_vnops.c | 10 +++++++--- tests/sys/fs/fusefs/flush.cc | 12 ++++++++++- tests/sys/fs/fusefs/locks.cc | 45 +++++++++++++++++++++++++++++++++++++++++- tests/sys/fs/fusefs/release.cc | 12 ++++++++++- 4 files changed, 73 insertions(+), 6 deletions(-) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=32be2332d10f6423def9d4c41d7e7707a97abae6 commit 32be2332d10f6423def9d4c41d7e7707a97abae6 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2021-03-18 20:27:27 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2021-04-08 02:15:42 +0000 fusefs: fix two bugs regarding fcntl file locks 1) F_SETLKW (blocking) operations would be sent to the FUSE server as F_SETLK (non-blocking). 2) Release operations, F_SETLK with lk_type = F_UNLCK, would simply return EINVAL. PR: 253500 Reported by: John Millikin <jmillikin@gmail.com> (cherry picked from commit 929acdb19acb67cc0e6ee5439df98e28a84d4772) sys/fs/fuse/fuse_vnops.c | 10 +++++++--- tests/sys/fs/fusefs/flush.cc | 12 ++++++++++- tests/sys/fs/fusefs/locks.cc | 45 +++++++++++++++++++++++++++++++++++++++++- tests/sys/fs/fusefs/release.cc | 12 ++++++++++- 4 files changed, 73 insertions(+), 6 deletions(-) A commit in branch stable/12 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=9a9c9e744b51e00f3446afb922500e40845cddfa commit 9a9c9e744b51e00f3446afb922500e40845cddfa Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2021-03-18 20:27:27 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2021-04-08 21:33:55 +0000 fusefs: fix two bugs regarding fcntl file locks 1) F_SETLKW (blocking) operations would be sent to the FUSE server as F_SETLK (non-blocking). 2) Release operations, F_SETLK with lk_type = F_UNLCK, would simply return EINVAL. PR: 253500 Reported by: John Millikin <jmillikin@gmail.com> (cherry picked from commit 929acdb19acb67cc0e6ee5439df98e28a84d4772) fusefs: fix a dead store in fuse_vnop_advlock kevans actually caught this in the original review and I fixed it, but then I committed an older copy of the branch. Whoops. Reported by: kevans Differential Revision: https://reviews.freebsd.org/D29031 (cherry picked from commit 9c5aac8f2e84ca4bbdf82514302c08c0453ec59b) sys/fs/fuse/fuse_vnops.c | 9 ++++++--- tests/sys/fs/fusefs/flush.cc | 12 ++++++++++- tests/sys/fs/fusefs/locks.cc | 45 +++++++++++++++++++++++++++++++++++++++++- tests/sys/fs/fusefs/release.cc | 12 ++++++++++- 4 files changed, 72 insertions(+), 6 deletions(-) *** Bug 255945 has been marked as a duplicate of this bug. *** |
Created attachment 222425 [details] quick-n-dirty patch to fix fcntl locking in FUSE There's at least two bugs in fcntl lock handling that get tickled by FUSE: 1. Calls to `fcntl(fd, F_SETLKW, &lock)` are showing up in the FUSE server as a FUSE_SETLK (should be FUSE_SETLKW). 2. fcntl calls to _clear_ a lock, by using F_SETLK with `l_type = F_UNLCK`, never get passed to userspace at all. It looks like both incorrect behaviors are caused by `kern_fcntl()`: * F_SETLKW is handled by a switch-case fallthrough to the F_SETLK path, but the `VOP_ADVLOCK()' call is hardcoded to use `F_SETLK'. * In the nested switch over the lock type, several calls to `VOP_ADVLOCK()' are performed using the lock type (e.g. `F_UNLCK') where they should use the fcntl opcode. This causes `fuse_vnop_advlock()' to return EINVAL. Attached is a patch that fixes both issues according to my test suite. I'm not very familiar with the FreeBSD kernel, so note that this patch might not do the right thing for other filesystem drivers.