Re: [PATCH v3] tty: tty_io: remove hung_up_tty_fops

From: Marco Elver <elver@google.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	 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>,
	"Paul E. McKenney" <paulmck@kernel.org>
Subject: Re: [PATCH v3] tty: tty_io: remove hung_up_tty_fops
Date: Mon, 29 Apr 2024 15:55:25 +0200	[thread overview]
Message-ID: <CANpmjNN250UCxsWCpUHAvJo28Lzv=DN-BKTmjEpcLFOCA+U+pw@mail.gmail.com> (raw)
In-Reply-To: <CAHk-=wj6HmDetTDhNNUNcAXZzmCv==oHk22_kVW4znfO-HuMnA@mail.gmail.com>

On Sun, 28 Apr 2024 at 20:50, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sun, 28 Apr 2024 at 03:20, Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >
> > If we keep the current model, WRITE_ONCE() is not sufficient.

FWIW, the original report here came from syzbot, which is configured
so that the WRITE_ONCE() is sufficient
(CONFIG_KCSAN_REPORT_RACE_UNKNOWN_ORIGIN=n,
CONFIG_KCSAN_IGNORE_ATOMICS=y ... long names, I know), because this
was an idiom we ran into in the past and just wanted to filter them
out (for better or worse).

That being said, the reader side still has a real problem, even if
hidden in some KCSAN configs. Up to you if the WRITE_ONCE() is
sufficient or not, at least on syzbot this case wouldn't resurface
(for now).

> > My understanding is that KCSAN's report like
>
> I find it obnoxious that these are NOT REAL PROBLEMS.
>
> It's KCSAN that is broken and doesn't allow us to just tell it to
> sanely ignore things.
>
> I don't want to add stupid and pointless annotations for a broken tooling.

KCSAN is the messenger in this case that our mental model vs. what our
language/memory model gives us is wrong: we already have our own
memory model (!= C11), but we still have data races, and have to deal
with the fallout. Data races (here: 2 plain unmarked accesses of the
pointers) still exist, and the compiler is still free to optimize them
(miscompile them according to our mental model).

Assuming the data race is not a problem assumes all compilers on all
architectures won't mess up the accesses.

This comes up over and over, and the problem hasn't gone away. Our
compilers still don't know about the kernel doing things outside the
scope of standard C - we can beat the compiler into submission with
lots of flags, but we know [1] compilers break our assumptions. What's
the long-term fix? I don't know, besides trying to teach compilers
more of what Linux wants.

[1] https://lpc.events/event/16/contributions/1174/attachments/1108/2121/Status%20Report%20-%20Broken%20Dependency%20Orderings%20in%20the%20Linux%20Kernel.pdf

> Can you instead just ask the KCSAN people to have some mode where we
> can annotate a pointer as a "use one or the other", and just shut that
> thing up that way?

Saying "use one or the other" pointer, implies atomicity of the load
and store. Telling KCSAN about our assumption of what we think the
compiler should do is the wrong way around - we should tell the
compiler. Which part of the code is telling the compiler we want
atomicity of the loaded pointers?

A WRITE_ONCE() / READ_ONCE() pair would do it here. What should we use instead?

Thanks,
-- Marco