Bug 266886 - [FUSEFS] Handling of l_whence with fcntl(F_GETLK)
Summary: [FUSEFS] Handling of l_whence with fcntl(F_GETLK)
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Alan Somers
URL: https://reviews.freebsd.org/D37014
Keywords:
Depends on:
Blocks:
 
Reported: 2022-10-07 10:49 UTC by John Millikin
Modified: 2022-10-31 01:51 UTC (History)
3 users (show)

See Also:
asomers: mfc-stable13+
asomers: mfc-stable12+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John Millikin 2022-10-07 10:49:07 UTC
I'm not 100% sure this is a bug, but it's a difference in behavior from FUSE on Linux, and IMO the Linux version's behavior is more correct here.

When using fcntl(F_GETLK) to check for the presence of advisory locks, it's possible to set the flock structure's `l_whence` field to `SEEK_CUR` or `SEEK_END` to adjust how the `l_start` and `l_end` fields are interpreted. For example, `SEEK_CUR` means they are relative to the file's current position.

FUSE doesn't pass through `l_whence`, so the Linux kernel does the adjustment itself when generating the FUSE request. When parsing the response, it overwrites the flock structure's `l_whence` field to `SEEK_SET` to signal that absolute positions are being reported. 

FreeBSD's implementation of FUSE_GETLK doesn't have this behavior, it passes through the start and end offsets as-is in both directions. This causes FUSE mounts to have abberent advisory lock reporting for binaries that depend on the Linux behavior.

(all of the above applies to F_SETLK / F_SETLKW also)
Comment 1 Alan Somers freebsd_committer freebsd_triage 2022-10-07 16:56:43 UTC
I can't reproduce this problem.  If I do the following:

	ASSERT_NE(-1, lseek(fd, 500, SEEK_SET));

	fl.l_start = 0;
	fl.l_len = 10;
	fl.l_pid = 42;
	fl.l_type = F_RDLCK;
	fl.l_whence = SEEK_CUR;
	fl.l_sysid = 42;
	ASSERT_NE(-1, fcntl(fd, F_GETLK, &fl)) << strerror(errno);

Then the server gets a FUSE_GETLK operation with start set to 500 and end set to 509, as it ought to.

I can't reproduce it with FUSE_SETLK either.

Could you help provide some more complete reproduction steps?
Comment 2 John Millikin 2022-10-12 04:03:50 UTC
I think my initial message was partially incorrect -- the offsets are being calculated correctly, it's just that `l_whence` doesn't get set to `SEEK_SET` when the call is complete.

	/* &fl populated per your example */
	fcntl(fd, F_GETLK, &fl));

	/* fuse server request and response
	RECV fuse_lk_in  { lk: fuse_file_lock { start: 500, end: 509, .. }, .. }
	SEND fuse_lk_out { lk: fuse_file_lock { start: 500, end: 509, .. } }
	*/

	ASSERT_EQ(fl.l_start, 500);
	ASSERT_EQ(fl.l_len, 10);
	ASSERT_EQ(fl.l_whence, SEEK_SET); /* fails */

In other words, I would expect one of the following to be true after fcntl returns:

1. l_start is an absolute location within the file, and l_whence is SEEK_SET.
2. l_start is relative to the file position, and l_whence is SEEK_CUR.
Comment 3 Alan Somers freebsd_committer freebsd_triage 2022-10-17 03:37:13 UTC
(In reply to John Millikin from comment #2)
Ok, I agree.  As the man page says, l_whence should be set to SEEK_SET on return from a successful F_GETLK operation.  After all, it's describing some other lock, not the one you queried about.  However, I don't see any reason why l_whence should be set to SEEK_SET on return from F_SETLK or F_SETLKW.

(There's a separate problem with SEEK_END and F_SETLK, which I'll fix.  fusefs doesn't correct the offset passed to the daemon.)
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-10-17 13:11:11 UTC
A commit in branch main references this bug:

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

commit 3c3b906b54236841d813fd9a01b1e090f39558ea
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-10-11 23:00:07 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-10-17 13:09:50 +0000

    fusefs: After successful F_GETLK, l_whence should be SEEK_SET

    PR:             266886
    Reported by:    John Millikin <jmillikin@gmail.com>
    MFC after:      2 weeks
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D37014

 sys/fs/fuse/fuse_vnops.c     |  1 +
 tests/sys/fs/fusefs/locks.cc | 69 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 2 deletions(-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2022-10-20 16:26:41 UTC
A commit in branch stable/12 references this bug:

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

commit 311f68079ff29aabd594c6fbdc97c2bff5d91ba5
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-10-11 23:00:07 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-10-20 16:24:19 +0000

    fusefs: After successful F_GETLK, l_whence should be SEEK_SET

    PR:             266886
    Reported by:    John Millikin <jmillikin@gmail.com>
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D37014

    (cherry picked from commit 3c3b906b54236841d813fd9a01b1e090f39558ea)

    fusefs: fix VOP_ADVLOCK with SEEK_END

    When the user specifies SEEK_END, unlike SEEK_CUR, VOP_ADVLOCK must
    adjust lock offsets itself.

    Sort-of related to bug 266886.

    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D37040

    (cherry picked from commit f6e5319550f60170840f1a07a9cbdd45b5014a21)

    Approved by:    kib (re)

 sys/fs/fuse/fuse_vnops.c      |  34 +++++++-
 tests/sys/fs/fusefs/locks.cc  | 191 +++++++++++++++++++++++++++++++++++++++++-
 tests/sys/fs/fusefs/mockfs.cc |  14 +++-
 3 files changed, 234 insertions(+), 5 deletions(-)
Comment 6 Graham Perrin freebsd_committer freebsd_triage 2022-10-29 19:31:28 UTC
Triage: 

* assign to committer resolving

* asomers@, attention to flags?
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-10-31 01:51:08 UTC
A commit in branch stable/13 references this bug:

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

commit faea785d30533566fa03807cdbd8a49d5fd16254
Author:     Alan Somers <asomers@FreeBSD.org>
AuthorDate: 2022-10-11 23:00:07 +0000
Commit:     Alan Somers <asomers@FreeBSD.org>
CommitDate: 2022-10-31 00:51:33 +0000

    fusefs: After successful F_GETLK, l_whence should be SEEK_SET

    PR:             266886
    Reported by:    John Millikin <jmillikin@gmail.com>
    Reviewed by:    emaste
    Differential Revision: https://reviews.freebsd.org/D37014

    (cherry picked from commit 3c3b906b54236841d813fd9a01b1e090f39558ea)

 sys/fs/fuse/fuse_vnops.c     |  1 +
 tests/sys/fs/fusefs/locks.cc | 69 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 68 insertions(+), 2 deletions(-)