Messages in this thread | | | From | Linus Torvalds <> | Date | Sat, 27 Apr 2024 12:02:32 -0700 | Subject | Re: [PATCH v3] tty: tty_io: remove hung_up_tty_fops |
| |
On Fri, 26 Apr 2024 at 23:21, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > syzbot is reporting data race between __tty_hangup() and __fput(), for > filp->f_op readers are not holding tty->files_lock.
Hmm. I looked round, and we actually have another case of this: snd_card_disconnect() also does
mfile->file->f_op = &snd_shutdown_f_ops;
and I don't think tty->files_lock (or, in the sound case, &card->files_lock) is at all relevant, since the users of f_ops don't use it or care.
That said, I really think we'd be better off just keeping the current model, and have the "you get one or the other". For the two cases that do this, do that f_op replacement with a WRITE_ONCE(), and just make the rule be that you have to have all the same ops in both the original and the shutdown version.
I do *not* think it's at all better to replace (in two different places) the racy f_op thing with another racy 'hungup' flag.
The sound case is actually a bit more involved, since it tries to deal with module counts. That looks potentially bogus. It does
fops_get(mfile->file->f_op);
after it has installed the snd_shutdown_f_ops, but in snd_open() it has done the proper
replace_fops(file, new_fops);
which actually drops the module count for the old one. So the sound case seems to possibly leak a module ref on disconnect. That's a separate issue, though.
Linus
Linus
| |