All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Alexander Potapenko <glider@google.com>
Cc: Matthew Wilcox <willy@infradead.org>,
	Segher Boessenkool <segher@kernel.crashing.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Alexei Starovoitov <ast@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andrey Konovalov <andreyknvl@google.com>,
	Andy Lutomirski <luto@kernel.org>, Arnd Bergmann <arnd@arndb.de>,
	Borislav Petkov <bp@alien8.de>, Christoph Hellwig <hch@lst.de>,
	Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Dmitry Vyukov <dvyukov@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Ilya Leoshkevich <iii@linux.ibm.com>,
	Ingo Molnar <mingo@redhat.com>, Jens Axboe <axboe@kernel.dk>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Kees Cook <keescook@chromium.org>, Marco Elver <elver@google.com>,
	Mark Rutland <mark.rutland@arm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Pekka Enberg <penberg@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Vegard Nossum <vegard.nossum@oracle.com>,
	Vlastimil Babka <vbabka@suse.cz>,
	kasan-dev <kasan-dev@googlegroups.com>,
	Linux Memory Management List <linux-mm@kvack.org>,
	Linux-Arch <linux-arch@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v4 44/45] mm: fs: initialize fsdata passed to write_begin/write_end interface
Date: Thu, 25 Aug 2022 09:33:18 -0700	[thread overview]
Message-ID: <CAHk-=wg-LXL4ZDMveCf9M7gWWwCMDG1dHCjD7g1u_vUXsU6Bzw@mail.gmail.com> (raw)
In-Reply-To: <CAG_fn=VbvbYVPfdKXrYRTq7HwmvXPQUeUDWZjwe8x8W=ttq6KA@mail.gmail.com>

On Thu, Aug 25, 2022 at 8:40 AM Alexander Potapenko <glider@google.com> wrote:
>
> On Mon, Jul 4, 2022 at 10:07 PM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > ... wait, passing an uninitialised variable to a function *which doesn't
> > actually use it* is now UB?  What genius came up with that rule?  What
> > purpose does it serve?
> >
>
> There is a discussion at [1], with Segher pointing out a reason for
> this rule [2] and Linus requesting that we should be warning about the
> cases where uninitialized variables are passed by value.

I think Matthew was actually more wondering how that UB rule came to be.

Personally, I pretty much despise *all* cases of "undefined behavior",
but "uninitialized argument" across a function call is one of the more
understandable ones.

For one, it's a static sanity checking issue: if function call
arguments can be uninitialized random garbage on the assumption that
the callee doesn't necessarily _use_ them, then any static checker is
going to be unhappy because it means that it can never assume that
incoming arguments have been initialized either.

Of course, that's always true for any pointer passing, but hey, at
least then it's pretty much explicit. You're passing a pointer to some
memory to another function, it's always going to be a bit ambiguous
who is supposed to initialize it - the caller or the callee.

Because one very important "static checker" is the person reading the
code. When I read a function definition, I most definitely have the
expectation that the caller has initialized all the arguments.

So I actually think that "human static checker" is a really important
case. I do not think I'm the only one who expects incomping function
arguments to have values.

But I think the immediate cause of it on a compiler side was basically
things like poison bits. Which are a nice debugging feature, even
though (sadly) I don't think they are usually added the for debugging.
It's always for some other much more nefarious reason (eg ia64 and
speculative loads weren't for "hey, this will help people find bugs",
but for "hey, our architecture depends on static scheduling tricks
that aren't really valid, so we have to take faults late").

Now, imagine you're a compiler, and you see a random incoming integer
argument, and you can't even schedule simple arithmetic expressions on
it early because you don't know if the caller initialized it or not,
and it might cause some poison bit fault...

So you'd most certainly want to know that all incoming arguments are
actually valid, because otherwise you can't do even some really simple
and obvious optimziations.

Of course, on normal architectures, this only ever happens with FP
values, and it's often hard to trigger there too. But you most
definitely *could* see it.

I personally was actually surprised compilers didn't warn for "you are
using an uninitialized value" for a function call argument, because I
mentally consider function call arguments to *be* a use of a value.

Except when the function is inlined, and then it's all different - the
call itself goes away, and I *expect* the compiler to DTRT and not
"use" the argument except when it's used inside the inlined function.

Because hey, that's literally the whole point of inlining, and it
makes the "static checking" problem go away at least for a compiler.

                     Linus

  reply	other threads:[~2022-08-25 16:33 UTC|newest]

Thread overview: 147+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-01 14:22 [PATCH v4 00/45] Add KernelMemorySanitizer infrastructure Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 01/45] x86: add missing include to sparsemem.h Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 02/45] stackdepot: reserve 5 extra bits in depot_stack_handle_t Alexander Potapenko
2022-07-12 14:17   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 03/45] instrumented.h: allow instrumenting both sides of copy_from_user() Alexander Potapenko
2022-07-12 14:17   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 04/45] x86: asm: instrument usercopy in get_user() and __put_user_size() Alexander Potapenko
2022-07-02  3:47   ` kernel test robot
2022-07-15 14:03     ` Alexander Potapenko
2022-07-15 14:03       ` Alexander Potapenko
2022-07-02 10:45   ` kernel test robot
2022-07-15 16:44     ` Alexander Potapenko
2022-07-15 16:44       ` Alexander Potapenko
2022-07-02 13:09   ` kernel test robot
2022-07-07 10:13   ` Marco Elver
2022-08-07 17:33     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 05/45] asm-generic: instrument usercopy in cacheflush.h Alexander Potapenko
2022-07-12 14:17   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 06/45] kmsan: add ReST documentation Alexander Potapenko
2022-07-07 12:34   ` Marco Elver
2022-07-15  7:42     ` Alexander Potapenko
2022-07-15  8:52       ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 07/45] kmsan: introduce __no_sanitize_memory and __no_kmsan_checks Alexander Potapenko
2022-07-12 14:17   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 08/45] kmsan: mark noinstr as __no_sanitize_memory Alexander Potapenko
2022-07-12 14:17   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 09/45] x86: kmsan: pgtable: reduce vmalloc space Alexander Potapenko
2022-07-11 16:12   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 10/45] libnvdimm/pfn_dev: increase MAX_STRUCT_PAGE_SIZE Alexander Potapenko
2022-07-11 16:26   ` Marco Elver
2022-08-03  9:41     ` Alexander Potapenko
2022-08-03  9:44     ` Alexander Potapenko
2023-01-05 22:08       ` Dan Williams
2023-01-09  9:51         ` Alexander Potapenko
2023-01-09 22:06           ` Dan Williams
2023-01-10  5:56             ` Greg Kroah-Hartman
2023-01-10  6:55               ` Dan Williams
2023-01-10  8:48                 ` Alexander Potapenko
2023-01-10  8:52                   ` Alexander Potapenko
2023-01-10  8:53                   ` Eric Dumazet
2023-01-10  8:55                     ` Christoph Hellwig
2023-01-10 15:35                       ` Steven Rostedt
2023-01-10  9:14                     ` Alexander Potapenko
2023-01-30  8:34         ` Alexander Potapenko
2023-01-30 18:57           ` Dan Williams
2022-07-01 14:22 ` [PATCH v4 11/45] kmsan: add KMSAN runtime core Alexander Potapenko
2022-07-02  0:18   ` Hillf Danton
2022-08-03 17:25     ` Alexander Potapenko
2022-07-11 16:49   ` Marco Elver
2022-08-03 18:14     ` Alexander Potapenko
2022-07-13 10:04   ` Marco Elver
2022-08-03 17:45     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 12/45] kmsan: disable instrumentation of unsupported common kernel code Alexander Potapenko
2022-07-12 11:54   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 13/45] MAINTAINERS: add entry for KMSAN Alexander Potapenko
2022-07-12 12:06   ` Marco Elver
2022-08-02 16:39     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 14/45] mm: kmsan: maintain KMSAN metadata for page operations Alexander Potapenko
2022-07-12 12:20   ` Marco Elver
2022-08-03 10:30     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 15/45] mm: kmsan: call KMSAN hooks from SLUB code Alexander Potapenko
2022-07-12 13:13   ` Marco Elver
2022-08-02 16:31     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 16/45] kmsan: handle task creation and exiting Alexander Potapenko
2022-07-12 13:17   ` Marco Elver
2022-08-02 15:47     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 17/45] init: kmsan: call KMSAN initialization routines Alexander Potapenko
2022-07-12 14:05   ` Marco Elver
2022-08-02 20:07     ` Alexander Potapenko
2022-08-03  9:08       ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 18/45] instrumented.h: add KMSAN support Alexander Potapenko
2022-07-12 13:51   ` Marco Elver
2022-08-03 11:17     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 19/45] kmsan: unpoison @tlb in arch_tlb_gather_mmu() Alexander Potapenko
2022-07-13  9:28   ` Marco Elver
2022-07-01 14:22 ` [PATCH v4 20/45] kmsan: add iomap support Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 21/45] Input: libps2: mark data received in __ps2_command() as initialized Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 22/45] dma: kmsan: unpoison DMA mappings Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 23/45] virtio: kmsan: check/unpoison scatterlist in vring_map_one_sg() Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 24/45] kmsan: handle memory sent to/from USB Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 25/45] kmsan: add tests for KMSAN Alexander Potapenko
2022-07-12 14:16   ` Marco Elver
2022-08-02 17:29     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 26/45] kmsan: disable strscpy() optimization under KMSAN Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 27/45] crypto: kmsan: disable accelerated configs " Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 28/45] kmsan: disable physical page merging in biovec Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 29/45] block: kmsan: skip bio block merging logic for KMSAN Alexander Potapenko
2022-07-13 10:22   ` Marco Elver
2022-08-02 17:47     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 30/45] kcov: kmsan: unpoison area->list in kcov_remote_area_put() Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 31/45] security: kmsan: fix interoperability with auto-initialization Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 32/45] objtool: kmsan: list KMSAN API functions as uaccess-safe Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 33/45] x86: kmsan: disable instrumentation of unsupported code Alexander Potapenko
2022-07-12 13:43   ` Marco Elver
2022-08-03 10:52     ` Alexander Potapenko
2022-07-01 14:22 ` [PATCH v4 34/45] x86: kmsan: skip shadow checks in __switch_to() Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 35/45] x86: kmsan: handle open-coded assembly in lib/iomem.c Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 36/45] x86: kmsan: use __msan_ string functions where possible Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 37/45] x86: kmsan: sync metadata pages on page fault Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 38/45] x86: kasan: kmsan: support CONFIG_GENERIC_CSUM on x86, enable it for KASAN/KMSAN Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 39/45] x86: fs: kmsan: disable CONFIG_DCACHE_WORD_ACCESS Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 40/45] x86: kmsan: don't instrument stack walking functions Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 41/45] entry: kmsan: introduce kmsan_unpoison_entry_regs() Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 42/45] bpf: kmsan: initialize BPF registers with zeroes Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 43/45] namei: initialize parameters passed to step_into() Alexander Potapenko
2022-07-02 17:23   ` Linus Torvalds
2022-07-03  3:59     ` Al Viro
2022-07-04  2:52     ` Al Viro
2022-07-04  8:20       ` Alexander Potapenko
2022-07-04 13:44         ` Al Viro
2022-07-04 13:55           ` Al Viro
2022-07-04 15:49           ` Alexander Potapenko
2022-07-04 16:03             ` Greg Kroah-Hartman
2022-07-04 16:33               ` Alexander Potapenko
2022-07-04 18:23             ` Segher Boessenkool
2022-07-04 16:00           ` Al Viro
2022-07-04 16:47             ` Alexander Potapenko
2022-07-04 17:36       ` Linus Torvalds
2022-07-04 19:02         ` Al Viro
2022-07-04 19:16           ` Linus Torvalds
2022-07-04 19:55             ` Al Viro
2022-07-04 20:24               ` Linus Torvalds
2022-07-04 20:46                 ` Al Viro
2022-07-04 20:51                   ` Linus Torvalds
2022-07-04 21:04                     ` Al Viro
2022-07-04 23:13                       ` [PATCH 1/7] __follow_mount_rcu(): verify that mount_lock remains unchanged Al Viro
2022-07-04 23:14                         ` [PATCH 2/7] follow_dotdot{,_rcu}(): change calling conventions Al Viro
2022-07-04 23:14                         ` [PATCH 3/7] namei: stash the sampled ->d_seq into nameidata Al Viro
2022-07-04 23:15                         ` [PATCH 4/7] step_into(): lose inode argument Al Viro
2022-07-04 23:15                         ` [PATCH 5/7] follow_dotdot{,_rcu}(): don't bother with inode Al Viro
2022-07-04 23:16                         ` [PATCH 6/7] lookup_fast(): " Al Viro
2022-07-04 23:17                         ` [PATCH 7/7] step_into(): move fetching ->d_inode past handle_mounts() Al Viro
2022-07-04 23:19                         ` [PATCH 1/7] __follow_mount_rcu(): verify that mount_lock remains unchanged Al Viro
2022-07-05  0:06                           ` Linus Torvalds
2022-07-05  3:48                             ` Al Viro
2022-07-04 20:47                 ` [PATCH v4 43/45] namei: initialize parameters passed to step_into() Linus Torvalds
2022-08-08 16:37   ` Alexander Potapenko
2022-07-01 14:23 ` [PATCH v4 44/45] mm: fs: initialize fsdata passed to write_begin/write_end interface Alexander Potapenko
2022-07-04 20:07   ` Matthew Wilcox
2022-07-04 20:30     ` Al Viro
2022-08-25 15:39     ` Alexander Potapenko
2022-08-25 16:33       ` Linus Torvalds [this message]
2022-08-25 21:57         ` Segher Boessenkool
2022-08-26 19:41           ` Linus Torvalds
2022-08-31 13:32             ` Alexander Potapenko
2022-08-25 22:13         ` Segher Boessenkool
2022-07-01 14:23 ` [PATCH v4 45/45] x86: kmsan: enable KMSAN builds for x86 Alexander Potapenko

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-=wg-LXL4ZDMveCf9M7gWWwCMDG1dHCjD7g1u_vUXsU6Bzw@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=arnd@arndb.de \
    --cc=ast@kernel.org \
    --cc=axboe@kernel.dk \
    --cc=bp@alien8.de \
    --cc=cl@linux.com \
    --cc=dvyukov@google.com \
    --cc=edumazet@google.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=gor@linux.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hch@lst.de \
    --cc=herbert@gondor.apana.org.au \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=iii@linux.ibm.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=keescook@chromium.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=luto@kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=mst@redhat.com \
    --cc=penberg@kernel.org \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=rientjes@google.com \
    --cc=rostedt@goodmis.org \
    --cc=segher@kernel.crashing.org \
    --cc=tglx@linutronix.de \
    --cc=vbabka@suse.cz \
    --cc=vegard.nossum@oracle.com \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /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.