Messages in this thread Patch in this message | | | From | Linus Torvalds <> | Date | Mon, 25 Nov 2019 19:21:56 -0800 | Subject | Re: Commit 0be0ee71 ("fs: properly and reliably lock f_pos in fdget_pos()") breaking userspace |
| |
On Mon, Nov 25, 2019 at 5:58 PM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > It might still be a bit noisy even with the above, but I think it will > at least be better.
Yeah, I think I see what's going on.
I for some reason entirely missed the tty case. Oops. That was just stupid of me, I should have thought of it.
There are other things that trigger the new informational line in that hacky patch, and they _might_ matter, but I suspect it's the tty and sound cases that causes the worst problems.
I suspect Kirill Smelkov even might have mentioned the tty case at one point, and I just spaced out.
There are other things too that trigger that debug check, like the sound file descriptors, and they might well matter too.
Anyway, I think the thing to do (for now) is to just say "character devices are FMODE_STREAM files if they have no llseek operations". That should take care of both tty's and the sound devices.
You can certainly have a character device that can do llseek, but it sounds like a reasonable base rule.
Of course, this may fix the f_pos locking issue, but replace it with a "oops, the character device driver tried to look at *pos anyway", and that will give you a nice OOPS instead.
So this patch might just replace the failure mode with another failure mode instead. At which point I think I'd have to revert that "get rid of FMODE_ATOMIC_POS" trial balloon, but let's see if this patch solves your problem and is sufficient..
I'd suggest using it _together_ with that "pr_info()" debug patch I sent, to see what else might be going on..
Linus fs/char_dev.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/fs/char_dev.c b/fs/char_dev.c index 00dfe17871ac..e5ffebdf80d5 100644 --- a/fs/char_dev.c +++ b/fs/char_dev.c @@ -367,6 +367,16 @@ void cdev_put(struct cdev *p) } } +static int might_be_stream(struct inode *inode, struct file *file) +{ + const struct file_operations *fops = file->f_op; + + if (fops->llseek && fops->llseek != no_llseek) + return 0; + + return stream_open(inode, file); +} + /* * Called every time a character special file is opened */ @@ -416,7 +426,7 @@ static int chrdev_open(struct inode *inode, struct file *filp) goto out_cdev_put; } - return 0; + return might_be_stream(inode, filp); out_cdev_put: cdev_put(p); | |