All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Dmitry Vyukov <dvyukov@google.com>,
	 syzbot <syzbot+b7c3ba8cdc2f6cf83c21@syzkaller.appspotmail.com>,
	 linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
	 Nathan Chancellor <nathan@kernel.org>,
	Arnd Bergmann <arnd@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	 Jiri Slaby <jirislaby@kernel.org>
Subject: Re: [PATCH v3] tty: tty_io: remove hung_up_tty_fops
Date: Sat, 27 Apr 2024 12:02:32 -0700	[thread overview]
Message-ID: <CAHk-=wgSOa_g+bxjNi+HQpC=6sHK2yKeoW-xOhb0-FVGMTDWjg@mail.gmail.com> (raw)
In-Reply-To: <18a58415-4aa9-4cba-97d2-b70384407313@I-love.SAKURA.ne.jp>

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

  reply	other threads:[~2024-04-27 19:02 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-21  8:18 [syzbot] [kernel?] KCSAN: data-race in __fput / __tty_hangup (4) syzbot
2023-04-21  8:21 ` Dmitry Vyukov
2023-04-21 15:12   ` Tetsuo Handa
2023-04-21 16:02     ` Tetsuo Handa
2023-04-23 23:34     ` Al Viro
2023-04-23 23:55       ` Tetsuo Handa
2023-04-24  0:44         ` Al Viro
2023-04-24  1:09           ` Tetsuo Handa
2023-04-25 14:47             ` Tetsuo Handa
2023-04-25 16:03               ` Al Viro
2023-04-25 22:09                 ` Tetsuo Handa
2023-04-26 11:05                   ` [PATCH] tty: tty_io: remove hung_up_tty_fops Tetsuo Handa
2023-04-28 16:27                     ` Nathan Chancellor
2023-04-28 16:41                       ` Tetsuo Handa
2023-04-28 17:11                         ` Al Viro
2023-04-29 10:43                           ` Tetsuo Handa
2023-04-28 17:31                         ` Greg Kroah-Hartman
2023-04-29 15:21                           ` Guenter Roeck
2023-05-01 18:42                             ` Geert Uytterhoeven
2023-05-14  1:02                     ` [PATCH v2] " Tetsuo Handa
2023-05-30 10:44                       ` Greg Kroah-Hartman
2023-05-30 11:57                         ` Tetsuo Handa
2023-05-30 12:51                           ` Greg Kroah-Hartman
2024-04-27  6:20                             ` [PATCH v3] " Tetsuo Handa
2024-04-27 19:02                               ` Linus Torvalds [this message]
2024-04-28 10:19                                 ` Tetsuo Handa
2024-04-28 18:50                                   ` Linus Torvalds
2024-04-29 13:55                                     ` Marco Elver
2024-04-29 15:38                                       ` Linus Torvalds
2024-05-01 18:45                                         ` Paul E. McKenney
2024-05-01 18:56                                           ` Linus Torvalds
2024-05-01 19:02                                             ` Paul E. McKenney
2024-05-01 20:14                                               ` Marco Elver
2024-05-01 21:06                                                 ` Linus Torvalds
2024-05-01 21:20                                                   ` Linus Torvalds
2024-05-01 21:49                                                     ` Paul E. McKenney
2024-05-01 22:32                                                       ` Paul E. McKenney
2024-05-02 16:37                                                         ` Boqun Feng
2024-05-03 23:59                                                           ` Paul E. McKenney
2024-05-04  0:14                                                             ` Linus Torvalds
2024-05-04  5:08                                                               ` Paul E. McKenney
2024-05-04 17:50                                                                 ` Linus Torvalds
2024-05-04 18:18                                                                   ` Paul E. McKenney
2024-05-04 19:11                                                                     ` Linus Torvalds
2024-05-04 19:25                                                                       ` Linus Torvalds
2024-05-04 22:17                                                                         ` Paul E. McKenney
2024-05-04 22:04                                                                       ` Paul E. McKenney
2024-05-02 14:14                                                   ` Marco Elver
2024-05-02 16:42                                                     ` Tetsuo Handa
2024-05-02 17:20                                                       ` Marco Elver
2024-05-02 17:29                                                       ` Linus Torvalds
2024-05-02 18:14                                                         ` Al Viro
2024-05-02 19:29                                                           ` Marco Elver
2024-05-02 23:54                                                         ` Tetsuo Handa
2024-05-03  1:12                                                           ` Linus Torvalds
2023-04-23 13:28   ` [syzbot] [kernel?] KCSAN: data-race in __fput / __tty_hangup (4) Tetsuo Handa
2023-04-23 14:00     ` Greg Kroah-Hartman
2023-04-23 14:03     ` Greg Kroah-Hartman
2023-04-23 14:17       ` Tetsuo Handa

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgSOa_g+bxjNi+HQpC=6sHK2yKeoW-xOhb0-FVGMTDWjg@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=arnd@kernel.org \
    --cc=dvyukov@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nathan@kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=syzbot+b7c3ba8cdc2f6cf83c21@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.