LKML Archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Jiri Slaby <jirislaby@kernel.org>,
	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
Subject: Re: [syzbot] [kernel?] KCSAN: data-race in __fput / __tty_hangup (4)
Date: Tue, 25 Apr 2023 17:03:44 +0100	[thread overview]
Message-ID: <20230425160344.GS3390869@ZenIV> (raw)
In-Reply-To: <2fca7932-5030-32c3-dd61-48dd78e58e11@I-love.SAKURA.ne.jp>

On Tue, Apr 25, 2023 at 11:47:20PM +0900, Tetsuo Handa wrote:
> @@ -743,7 +733,8 @@ void tty_vhangup_session(struct tty_struct *tty)
>   */
>  int tty_hung_up_p(struct file *filp)
>  {
> -	return (filp && filp->f_op == &hung_up_tty_fops);
> +	/* Accept race with __tty_hangup(). */
> +	return filp && data_race(((struct tty_file_private *) filp->private_data)->hung);
>  }
>  EXPORT_SYMBOL(tty_hung_up_p);

Umm...  Are you sure we never call that for non-TTY files?  Seeing that
it's exported and all such...  For internal uses (tty_read(), etc.) the
check for file being NULL is pointless; for general-purpose primitive
we probably want to check ->f_op as well...

> @@ -2159,11 +2154,6 @@ static int tty_open(struct inode *inode, struct file *filp)
>  			return retval;
>  
>  		schedule();
> -		/*
> -		 * Need to reset f_op in case a hangup happened.
> -		 */
> -		if (tty_hung_up_p(filp))
> -			filp->f_op = &tty_fops;

That needs a comment in commit message - hangup indication doesn't
need to be reset here, since tty_release() has freed the entire thing
that used to hang off the ->private_data.

> I don't know what changes are required for tty_open() and tty_show_fdinfo().
> I assume that making no change for tty_show_fdinfo() is harmless.
> But how does "hung_up_tty_fops does not call tty_open()" affects here?

It doesn't - file->f_op->open() is only called once, and only on fresh
struct file.

  reply	other threads:[~2023-04-25 16:04 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 [this message]
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
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=20230425160344.GS3390869@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=dvyukov@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=syzbot+b7c3ba8cdc2f6cf83c21@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).