Bug 265736
| Summary: | F_SETFL will fail yet still set O_NONBLOCK for character devices that don't implement d_ioctl | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | Yuval Pavel Zholkover <paulzhol> | ||||||||
| Component: | kern | Assignee: | Alan Somers <asomers> | ||||||||
| Status: | Closed FIXED | ||||||||||
| Severity: | Affects Some People | CC: | asomers, eduardo, emaste, grahamperrin, paulzhol | ||||||||
| Priority: | --- | Flags: | asomers:
mfc-stable13+
asomers: mfc-stable12- |
||||||||
| Version: | 13.1-STABLE | ||||||||||
| Hardware: | Any | ||||||||||
| OS: | Any | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Yuval Pavel Zholkover
2022-08-09 13:39:27 UTC
Comment on attachment 235799 [details]
bazil/fuse workaround
With this patch applied, the file descriptor is never attempted to be put into nonblocking mode. And no EAGAIN is observed later which allows the program to mount the fusefs.
You attached that patch to the wrong bug. This bug is about fcntl's behavior with /dev/fuse . Patches to ports should go in the original rclone bug, or in a new one. (In reply to Alan Somers from comment #2) I've added it here because I think it helps with the reproducer (controlling the open flags directly without the netpoller). It is the same as the one in bug #258056 Opening /dev/fuse with O_NONBLOCK works fine. It just doesn't support switching into non-blocking mode after opening. I could add that feature, though. Would that help you? (In reply to Alan Somers from comment #4) Personally I think it is fine to not support changing the device after the initial open. My point about opening the bug is the side-effect of calling F_SETFL with O_NONBLOCK has: flow 1: open() blocking read() - puts the thread to sleep flow 2: open() O_NONBLOCK kqueue+read() when ready - returns EAGAIN, as expeced when no more data is available. flow 3: open() blocking fcntl(F_SETFL, O_NONBLOCK) - returning an error read() - returns EAGAIN, instead of putting the thread to sleep flow 3 shouldn't behave differently than flow 1 (other than the fcntl returning an error). Created attachment 243323 [details]
Standalone test case
This is not FUSE-specific. The bug is in kern_fcntl. It sets fp->f_flag to the new flags, then calls the file descriptor's ioctl handler. But if that fails, it doesn't return fp->f_flag to the original value. The ioctl will fail for any character device that doesn't implement .d_ioctl . /dev/fuse doesn't, and there are 32 others that don't either. One example is everything under /dev/led/* (from sys/dev/led/led.c).
Steps to Reproduce
==================
cc -O2 -Wall -g -o f_setfl_o_nonblock f_setfl_o_nonblock.c
./f_setfl_o_nonblock /dev/led/ahci0.0.locate
F_SETFL failed. Flags changed from 0 to 0x4.
./f_setfl_o_nonblock /dev/fuse
F_SETFL failed. Flags changed from 0 to 0x4.
Since F_SETFL failed, the flags should be unchanged.
Created attachment 243325 [details]
Better test case. It tests both setting and clearing flags.
Attached a better test case. It tests both setting and clearing flags.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=6c049996ec29bad4a913b019a28f211ab84b0d3d commit 6c049996ec29bad4a913b019a28f211ab84b0d3d Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2023-07-09 20:48:10 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2023-07-10 14:13:58 +0000 During F_SETFL, don't change file flags on error Previously, even if the FIONBIO or FIOASYNC ioctl failed, the file's f_flags variable would still be changed. Now, kern_fcntl will restore the original flags if the ioctl fails. PR: 265736 Reported by: Yuval Pavel Zholkover <paulzhol@gmail.com> MFC after: 2 weeks Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D40955 sys/kern/kern_descrip.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) A commit in branch stable/13 references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=14ca2bc402bb82fb59a9aede05f62cb5e634b710 commit 14ca2bc402bb82fb59a9aede05f62cb5e634b710 Author: Alan Somers <asomers@FreeBSD.org> AuthorDate: 2023-07-09 20:48:10 +0000 Commit: Alan Somers <asomers@FreeBSD.org> CommitDate: 2023-09-06 20:45:11 +0000 During F_SETFL, don't change file flags on error Previously, even if the FIONBIO or FIOASYNC ioctl failed, the file's f_flags variable would still be changed. Now, kern_fcntl will restore the original flags if the ioctl fails. PR: 265736 Reported by: Yuval Pavel Zholkover <paulzhol@gmail.com> Reviewed by: kib Differential Revision: https://reviews.freebsd.org/D40955 (cherry picked from commit 6c049996ec29bad4a913b019a28f211ab84b0d3d) sys/kern/kern_descrip.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) Not bothering to MFC to stable/12. Reopen if there's a need. |
